VOGONS

Critique my code! :P

Topic actions

First post, by keenmaster486

Posted on 
User metadata
Rank l33t
Rank
l33t

So for my Computer Science class, they assigned a project entitled "Design your own game".

Here's the writeup on it:

Design	your	own	game
In	this	project,	you	will	be	designing	your	own	interactive	computer	game.	Yep,	
designing your	own game.	The	idea	for	the game	and	the code	to	implement	your	
idea will	come from	you.	The	only	requirement for	the	assignment	is that	your	game	
include	the	following	programming	features:
At	least:
• 4	functions
• 2	loops	(either	while	or	for,	or	one	of	each)
• 4	if/elif/else	blocks
• 4	numerical	variables
• 4	strings
• File	I/O
o Reading	from	or	writing	to	a	file.	
• Console	I/O
o Ask	the	user	for	input	and	handle	that	input.
• 3	classes,	including
o At	least	one	base	and	one	derived	class
[etc...]
What	 is a game?
The	definition	of	computer	game	 for	this	assignment is	anything that	interacts	with	
the user and	continues	going	until	the	user	says	Quit.	Clearly,	this	is	a	more	relaxed	
definition	than	might	generally	be	used.	All	user	interaction	we	have	done	so	far	has	
been	command-line	based,	such	as	the “I’m	thinking	of	a	number	between	1	and	
100”	game	that we	did	early	in	the	semester. You	are	also	welcome	to	incorporate	
sophisticated sound and graphics.
Game	subject	areas
The	subject	area	of	your	game	is	also	your	choice.	If	you	want	to	write	an	
educational	game	that	quizzes	your	knowledge	of	chemistry,	sounds	great.	A	choose	
your	own	adventure	game?	This	is	also	a	fabulous	idea. Help	the	horse	navigate	a	
complex	maze?	Great.

So I immediately thought I'd like to try writing a 2D platformer, and then proceeded to put it off until yesterday 😵 😵 😵

So I literally whipped up this code in a sum total of about five or six hours between last night and this morning. I had never used SDL in my entire life before last night; neither had I ever done graphics programming in C before. So this code is not guaranteed to be good 🤣

I'm just looking for everyone's opinions on my programming practices on the whole. Since I had extremely limited time I did cut a lot of corners where I knew they shouldn't be, and I didn't comment very well. But I guess this is interesting because it's kind of a measure of what my base instincts as a programmer are; i.e. what I do when I'm up against the wire and don't have time to make anything but the quick and dirty solution.

Here's the project:

The attachment Project1.zip is no longer available

And here's the code if you don't want to mess with CodeBlocks:

#include <SDL2/SDL.h>
#include <stdio.h>
#include <iostream>
#include <fstream>
#include <string>
#include <cmath>

using namespace std;

//############# Project 1 for Computer Science 1310
//
//TA Name: #################
//
//
/*


This project is a demo of a 2D platform game engine.

It requires version 2.0 of the Simple Directmedia Layer Library,
otherwise known as SDL2. You can get this with the command

sudo apt-get install libsdl2-dev

if it's not already installed.

This program also requires the files

TILES.EGA
SPRITES.EGA
LEVEL01.MAP

to be in the current directory when it is run. Otherwise it won't work
since it needs these files for graphics and level data.

How to use the demo:

When you run the demo and skip the user input section at the beginning;
it loads the graphics and level and you're presented with the demo level.

You control a demo character with the following keys:

Arrow keys ----- move
Left ctrl ------ jump
Esc ------------ quit

He's not animated because I didn't have time, but the extra sprites are loaded
into memory anyway.

You can see that there are two total tiles right now; more could be added if needed.

The level does not scroll but the framework to do that is built in.

The keys are a little delayed; I'm not sure why. Something to debug later.

Press ESC to quit the demo.


Note on screen/window resolution:

…Show last 636 lines
The program, by default, emulates a resolution of 320x200 on a window size of 1280x800.

You can change this by the constants below. It's kind of fun to fiddle with it to see what you can do.

*/






//===========DECLARE GLOBAL VARIABLES==============
    //Screen dimension constants
    const float SCREEN_WIDTH = 1280;
    const float SCREEN_HEIGHT = 800;
    const float SCREEN_SIMWIDTH = 320;
    const float SCREEN_SIMHEIGHT = 200;

    const float SCREEN_WMULT = SCREEN_WIDTH/SCREEN_SIMWIDTH;
    const float SCREEN_HMULT = SCREEN_HEIGHT/SCREEN_SIMHEIGHT;

    const float gravity = 1; //acceleration due to gravity (in pixels per loop cycle per loop cycle)
    const float terminalv = 8; //terminal velocity due to air resistance

    //Loop delay to make sure it doesn't go too fast:
    int loopdelay = 20;

    //Main rendering window:
	SDL_Window* window = NULL;

	//Surface on the main rendering window:
	SDL_Surface* screenSurface = NULL;

	//Renderer:
    SDL_Renderer* gRenderer = NULL;

    //Event handler:
    SDL_Event events;

    //Main loop exit flag:d
    bool exitmainloop = false;

    //LEVEL DATA (just foreground tiles for now; leave room for 100x100 levels:
    int lx = 0;
    int ly = 0;
    int foretile[200][200];
    int midtile[200][200];
    int backtile[200][200];


    //FILES:
    ifstream myfile;



float dir(float temp)
{
    if (temp>0) {return 1;}
    if (temp==0) {return 0;}
    if (temp<0) {return -1;}
}

bool initSDL()
{
    //SDL initialization code:
	if( SDL_Init( SDL_INIT_VIDEO ) < 0 )
	{
		printf( "SDL could not initialize! SDL_Error: %s\n", SDL_GetError() );
		return false;
	}
	else
	{
		//Create window
		window = SDL_CreateWindow( "2D Platformer", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, SCREEN_WIDTH, SCREEN_HEIGHT, SDL_WINDOW_SHOWN );
		if( window == NULL )
		{
			printf( "Window could not be created! SDL_Error: %s\n", SDL_GetError() );
			return false;
		}
		else
		{
			//screenSurface = SDL_GetWindowSurface( window );
			gRenderer = SDL_CreateRenderer (window, -1, NULL);
			//Get window surface
			//screenSurface = SDL_GetWindowSurface( window );

			//Fill the surface white
			//SDL_FillRect( screenSurface, NULL, SDL_MapRGB( screenSurface->format, 0xFF, 0xFF, 0xFF ) );

			//Update the surface
			//SDL_UpdateWindowSurface( window );

			//Wait two seconds
			//SDL_Delay( 2000 );
			cout<<"SDL initialized successfully"<<endl;
			return true;
        }
	}
}

bool shutdownSDL()
{
    SDL_DestroyRenderer (gRenderer);
    //Destroy window
    SDL_DestroyWindow( window );

	//Quit SDL subsystems
	SDL_Quit();
	cout<<"SDL shutdown successfully"<<endl;
	return true;
}



void KGEpset(int x, int y, int c)
{
    //Convert c from a 16-color standard EGA palette to RGB
    //Why not use the VGA palette like everyone else, you say?
    //We do this because we like the EGA palette. It's cool.

    int red = 0x00;
    int green = 0x00;
    int blue = 0x00;
    switch (c)
    {
        //For reference:
        //0x00 = 0
        //0x55 = 85
        //0xAA = 170
        //0xFF = 255

        case 0: //black
            red = 0x00;
            green = 0x00;
            blue = 0x00;
            break;
        case 1: //blue
            red = 0x00;
            green = 0x00;
            blue = 0xAA;
            break;
        case 2: //green
            red = 0x00;
            green = 0xAA;
            blue = 0x00;
            break;
        case 3: //cyan
            red = 0x00;
            green = 0xAA;
            blue = 0xAA;
        case 4: //red
            red = 0xAA;
            green = 0x00;
            blue = 0x00;
            break;
        case 5: //magenta
            red = 0xAA;
            green = 0x00;
            blue = 0xAA;
            break;
        case 6: //brown
            red = 0xAA;
            green = 0x55;
            blue = 0x00;
            break;
        case 7: //gray
            red = 0xAA;
            green = 0xAA;
            blue = 0xAA;
            break;
        case 8: //dark gray
            red = 0x55;
            green = 0x55;
            blue = 0x55;
            break;
        case 9: //light blue
            red = 0x55;
            green = 0x55;
            blue = 0xFF;
            break;
        case 10: //light green
            red = 0x55;
            green = 0xFF;
            blue = 0x55;
            break;
        case 11: //light cyan
            red = 0x55;
            green = 0xFF;
            blue = 0xFF;
            break;
        case 12: //light red
            red = 0xFF;
            green = 0x55;
            blue = 0x55;
            break;
        case 13: //light magenta
            red = 0x0FF;
            green = 0x55;
            blue = 0xFF;
            break;
        case 14: //yellow
            red = 0xFF;
            green = 0xFF;
            blue = 0x55;
            break;
        case 15: //bright white
            red = 0xFF;
            green = 0xFF;
            blue = 0xFF;
            break;
    }
    //SDL_FillRect(screenSurface, NULL, SDL_MapRGB( screenSurface->format, red, green, blue) );
    //SDL_FillRect(screenSurface, NULL, SDL_MapRGB( screenSurface->format, 0xFF, 0xFF, 0xFF) );

                //SDL_Rect fillRect = {x*SCREEN_WMULT, y*SCREEN_HMULT, (x+1)*SCREEN_WMULT, (y+1)*SCREEN_HMULT };
                SDL_Rect fillRect = {x*SCREEN_WMULT, y*SCREEN_HMULT, SCREEN_WMULT, SCREEN_HMULT };
                SDL_SetRenderDrawColor( gRenderer, red, green, blue, SDL_ALPHA_OPAQUE );
                SDL_RenderFillRect( gRenderer, &fillRect );
}


class Graphic
{
public:
    int picdata[200][200];
    int xsize = 0;
    int ysize = 0;
protected:
private:
};
class Tile: public Graphic
{
public:
    int type = 0;
    int inf[5];
    int picdata[16][16];
    void drawTile(int x, int y, int page)
    {
        for (int j=x;j<=x+16;j++)
        {
            for (int k=y;k<=y+16;k++)
            {
                KGEpset(j,k,picdata[j-x][k-y]);
            }
        }
    }
protected:
private:
};
class Sprite: public Graphic
{
public:
    void drawSprite(int x, int y, int page)
    {
        for (int j=x;j<=x+xsize;j++)
        {
            for (int k=y;k<=y+ysize;k++)
            {
                if (picdata[j-x][k-y] > -1) {KGEpset(j,k,picdata[j-x][k-y]);}
            }
        }
    }
protected:
private:
};

//DECLARE GRAPHICS VARIABLES:

Tile tiles[100]; //Allow room for 100 tiles
Sprite sprites[200]; //Allow room for 200 sprites

class Object
{
public:
    int spr = 0;
    int xpos = 0;
    int ypos = 0;
    int xsize = 0;
    int ysize = 0;
    float xv = 0;
    float xa = 0;
    float yv = 0;
    float ya = 0;
    float grav = gravity;
    float fric = 0;
    void SetSprite(int s)
    {
        spr = s;
        xsize = sprites[spr].xsize;
        ysize = sprites[spr].ysize;
    }
    void SetPos(int t, int t2)
    {
        xpos = t;
        ypos = t2;
    }
    void SetXV(int t)
    {
        xv = t;
        fric = 0;
    }
    void SetXA(int t)
    {
        xa = t;
    }
    void SetYV(int t)
    {
        yv = t;
    }
    void SetYA(int t)
    {
        ya = t;
    }
    void SetGravity(int g)
    {
        grav = g;
    }
    void ResetGravity()
    {
        grav = gravity;
    }
    void SetFriction (int f)
    {
        fric = f;
    }
    void ResetFriction ()
    {
        fric = 0;
    }
    void Increment()
    {
        xa = abs(fric) * -dir(xv);
        if (abs(xv) < 1) {xa = 0;xv=0;}
        xv = xv + xa;
        ya = ya + grav;
        yv = yv + ya;
        if (yv>terminalv) {yv = terminalv;}
        xpos = xpos + xv;
        ypos = ypos + yv;
        if (xpos<0) {xpos=0;}
        if (ypos<0) {ypos=0;}
        if (xpos>(SCREEN_SIMWIDTH)-24) {xpos=SCREEN_SIMWIDTH-24;}
        if (ypos>SCREEN_SIMHEIGHT-32) {ypos=SCREEN_SIMHEIGHT-32;}
    }
    void Draw()
    {
        sprites[spr].drawSprite(xpos, ypos, 1);
    }
    friend void DoNothing();
protected:
private:

};

void DoNothing ()
{
    //This is a friend function to class Object since one was required. It doesn't do anything because I don't need it to do anything.
}

    //DECLARE OBJECTS:
    Object player;

bool coltop(Object o, int tx, int ty)
{
    //Check if an object is colliding with a particular tile on the top
    if ( (o.xpos + o.xsize > tx) && (o.xpos < tx+16) && (o.ypos + o.ysize > ty) && (o.ypos < ty+16) )
    {
        return true;
    }
    else
    {
        return false;
    }

}

void handlekeys()
{

        if(SDL_PollEvent(&events) != 0)
                    {
                        if (events.type == SDL_QUIT)
                        {
                            exitmainloop = true;
                        }
                        else if (events.type == SDL_KEYDOWN)
                        {
                            switch (events.key.keysym.sym)
                            {
                            case SDLK_ESCAPE:
                                exitmainloop = true;
                                break;
                            case SDLK_UP:
                                //do something for up key
                                break;
                            case SDLK_DOWN:
                                //do something for down key
                                break;
                            case SDLK_LEFT:
                                //do something for left key
                                player.SetXV(-6);
                                player.SetXA(0);
                                player.fric = 0;
                                break;
                            case SDLK_RIGHT:
                                //do something for right key
                                player.SetXV(6);
                                player.SetXA(0);
                                player.fric = 0;
                                break;
                            case SDLK_LCTRL:
                                //do something for left control key
                                //JUMP!
                                player.SetYV(-20);
                                break;
                            case SDLK_SPACE:
                                //do something for space key
                                break;
                            case SDLK_LALT:
                                //do something for left alt key
                                break;

                            ;
                            }
                        }
                    }
}

void displayLevel (int scrx, int scry)
{
    int cx = 0;
    int cy = 0;
    for (int j=0;j<=SCREEN_SIMWIDTH/16;j++)
    {
        for (int k=0;k<=SCREEN_SIMHEIGHT/16;k++)
        {
            cx = (j*16)-scrx;
            cy = (k*16)-scry;
            //KGEpset(cx,cy,15);
            //if (backtile[j][k] > -1)
            //{
            //    tiles[backtile[j][k]].drawTile(cx,cy,0);
            //}
            //if (midtile[j][k] > -1)
            //{
            //    tiles[midtile[j][k]].drawTile(cx,cy,0);
            //}
            //if (foretile[j][k] > 0)
            //{
                tiles[foretile[j][k]].drawTile(cx,cy,0);
            //}
            if ( (coltop(player, cx, cy)) && foretile[j][k] > 0 && player.yv >= 0)
            {
                player.ypos = cy-32;
                player.ya = 0;
                player.yv = 0;
                player.fric = 2;
            }
        }
    }
}


void mainloop()
{
    player.SetSprite(1);




			//Update the surface
			SDL_UpdateWindowSurface( window );
			SDL_RenderClear (gRenderer);
			SDL_RenderPresent (gRenderer);




                exitmainloop = false;
                while(!exitmainloop)
                {

                    SDL_Delay(loopdelay);




                    //sprites[1].drawSprite(0,0,0);
                    //tiles[1].drawTile(50,50,0);
                    displayLevel(0,0);
                    handlekeys();
                    player.Draw();
                    player.Increment();
                    player.ResetGravity();
                    player.ResetFriction();

                    SDL_RenderPresent (gRenderer);



                }

}





void LoadGraphics()
{
    //Loads graphics from files TILES.EGA and SPRITES.EGA:
    cout<<"Loading tiles..."<<endl;
    string temp = "";
    int temp2 = 0;
    cout<<"Opening file...";
    myfile.open("TILES.EGA");
    cout<<"Done."<<endl;
    cout<<"Reading number of tiles...";
    getline(myfile,temp,'\n');
    int tilenum = stoi(temp);
    cout<<"Done. There are "<<tilenum<<" tiles."<<endl;
    cout<<"Reading tiles";
    for (int i=1;i<=tilenum;i++)
    {
        for (int k=0;k<=15;k++)
        {
            for (int j=0;j<=15;j++)
            {
                getline(myfile,temp,',');
                tiles[i].picdata[j][k] = stoi(temp);
                //cout<<temp<<",";
            }
            //cout<<endl;
            cout<<".";
        }
        getline(myfile,temp,';');
    }
    cout<<"Done."<<endl;
    myfile.close();


    cout<<"Loading sprites..."<<endl;
    temp = "";
    temp2 = 0;
    cout<<"Opening file...";
    myfile.open("SPRITES.EGA");
    cout<<"Done."<<endl;
    cout<<"Reading number of tiles...";
    getline(myfile,temp,'\n');
    int sprnum = stoi(temp);
    cout<<"Done. There are "<<sprnum<<" sprites."<<endl;
    cout<<"Reading sprites";
    for (int i=1;i<=sprnum;i++)
    {
        //Get size of sprite:
        getline(myfile,temp,',');
        sprites[i].xsize = stoi(temp);
        getline(myfile,temp,'\n');
        sprites[i].ysize = stoi(temp);
        //Get actual graphics data:
        for (int j=0;j<=sprites[i].xsize;j++)
        {
            for (int k=0;k<=sprites[i].ysize;k++)
            {
                getline(myfile,temp,'\n');
                sprites[i].picdata[j][k] = stoi(temp);
            }
        }
        cout<<".";
    }
    cout<<"Done."<<endl;
    myfile.close();
}

void LoadLevel(int l)
{
    //Actually, the argument passed to this doesn't do anything right now since we only have one level.
    //SDL_Delay(5000);
    cout<<"Loading Level..."<<endl;
    cout<<"Opening file...";
    myfile.open ("LEVEL01.MAP");
    cout<<"Done."<<endl;
    string temp = "";
    cout<<"Getting level size...";
    getline(myfile,temp,',');
    lx = stoi(temp);
    getline(myfile,temp,'\n');
    ly = stoi(temp);
    cout<<"Done. The size is "<<lx<<"x"<<ly<<"."<<endl;
    cout<<"Getting level data";
    //for (int i=1;i<=tilenum;i++)
    //{
        for (int k=0;k<=lx-1;k++)
        {
            for (int j=0;j<=ly-1;j++)
            {
                getline(myfile,temp,',');
                foretile[j][k] = stoi(temp);
                //cout<<temp<<",";
            }
            //cout<<endl;
            cout<<".";
        }
        getline(myfile,temp,';');
    //}
    cout<<"Done."<<endl;
}




int main( int argc, char* args[] )
{

cout<<"Welcome to this demo of a 2D platformer."<<endl;
cout<<"Enter a value to delay the main loop by"<<endl;
cout<<"every time it comes around, to make sure"<<endl;
cout<<"things don't go too fast. The default value"<<endl;
cout<<"is 20. Just press enter to go to default."<<endl;
string blah="";
getline(cin,blah);
if (blah != "") {loopdelay = stoi(blah);}
    if (!initSDL()) {shutdownSDL();}

LoadGraphics();
LoadLevel(1);

    mainloop();



    shutdownSDL();

	return 0;
}

I know my "loop delay" thing is the dorkiest possible solution. I just didn't have time to implement a proper timer.

Thoughts, philosophical musings, anyone?

World's foremost 486 enjoyer.

Reply 1 of 3, by Jepael

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

I had to try - didn't compile until SDL2 was installed c++11 was enabled.

Nice - I've only ever used SDL for playing audio, because I could not think of easier way to play something I algorithmically generate (sound chip emulation) instead of being awkward and write raw audio to file and load it in audio editor.

Reply 2 of 3, by keenmaster486

Posted on 
User metadata
Rank l33t
Rank
l33t

Oh yeah, I forgot to mention it makes heavy use of C++11 functions.

I haven't tried the audio functions yet. Something to look at.

World's foremost 486 enjoyer.

Reply 3 of 3, by vladstamate

Posted on 
User metadata
Rank Oldbie
Rank
Oldbie

You asked for it!

- class Graphic.
Why is it a class? It has no member functions and all the variables are public. A structure would do. Now you have things like public, protected and private littering your code.

- class Tile/Sprite
Ahh so that is why Graphic is a class, so you can inherit? Might as well do it properly then. Create a pure virtual function draw inside Graphic and then each Tile and Sprite will actually implement it. That would be much cleaner than having separate drawTile/drawSprite as they have same signature.

- class Object
All your members are public yet you still have SetXXX functions. Why? Go one way or the other.

- Object player
Is indented for no reason. Good indentation practices makes code easier to read.

- unUniformly_nameFunctionsandvaribles
Yeah, You have ResetGravity and you also have handlekeys and also coltop (very bad name) and also displayLevel. You should try to stick to one naming scheme. The 3 main schemes are: 1) LikeThis 2) likeThis or 3) like_this

- a lot of strange indentations and extraneous ; in handlekeys

- commented out code should not really be there

- do not name variables "temp" or "temp2" as they do not mean anything. Or even worse blah.

- way too many new lines.

YouTube channel: https://www.youtube.com/channel/UC7HbC_nq8t1S9l7qGYL0mTA
Collection: http://www.digiloguemuseum.com/index.html
Emulator: https://sites.google.com/site/capex86/
Raytracer: https://sites.google.com/site/opaqueraytracer/

Go to top of page Go to top of page
Back to Milliways
Close window
Close window