# C++ game with splash screen and menus

As of now this game only consists of:

• Splash Screen

Did I apply bad practices, or did I do something well? Is there a better way to do it?

#include <iostream>
#include <fstream>///filestream lib-- needed for file i/o
#include <windows.h>///Needed for alt clear screen function
#include <iomanip>///setw-- for indentation on title screen or headers in game
#include <cstdlib>
#include <limits>///used for char -> int input error
#include <conio.h>/*Used for pause screen--Windows Specific-- MUST FIND PORTABLE SOLUTION, possibly ifdef*/
//#include <cctype>
#include <string>
#include <ctime>///used for initializer for now

/**********************************************/
///used to clear screen cross platform
///#ifdef _WIN32 //checks to see if windows OS
///#define CLEAR "cls"
///#else //In any other OS
///#define CLEAR "clear"
///#endif
/*********************************************/
int inputError();///custom error message at top of screen
void Clear();///Safer way to clear screen non system()
void SplashScreen();///Production logo
void Help();
void Game();
/********************************************/
int main(int arg, char** argv)
{
SplashScreen();///display Splash screen
Clear();///clear Splash and other text

MainMenu();///Pretty Much the start of the main Game loop
}
/***********************************************************************/
void Clear()
{
COORD topLeft  = { 0, 0 };
HANDLE console = GetStdHandle(STD_OUTPUT_HANDLE);///get a handle to the console
CONSOLE_SCREEN_BUFFER_INFO screen;
DWORD written;///num of characters written

GetConsoleScreenBufferInfo(console, &screen);

FillConsoleOutputCharacterA(
console, ' ', screen.dwSize.X * screen.dwSize.Y, topLeft, &written
);
///Overwrite console with whitespace
FillConsoleOutputAttribute(
console, FOREGROUND_GREEN | FOREGROUND_RED | FOREGROUND_BLUE,
screen.dwSize.X * screen.dwSize.Y, topLeft, &written
);
SetConsoleCursorPosition(console, topLeft);///reset cursor to topleft pos
}
/***********************************************************************/
int inputError()
{
HANDLE hStdout = GetStdHandle(STD_OUTPUT_HANDLE);
if (hStdout == INVALID_HANDLE_VALUE)
{
std::cout << "Error while getting input handle" << std::endl;
return 1;
}
///sets the color to intense red on blue background
SetConsoleTextAttribute(hStdout, FOREGROUND_RED | FOREGROUND_INTENSITY);
std::cout <<"Previous selection was invalid, try another from the list: " << std::endl;

///reverting back to the normal color
SetConsoleTextAttribute(hStdout, FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE);
}
/***********************************************************************/
void SplashScreen()
{
std::string Lines = "";///NULL string
std::ifstream Splash("TextFiles\\splash.txt");///ifstream used to read ascii art from txt

std::cout <<"Presented by...\n" << std::endl;
if(Splash.is_open())
{
while(!Splash.eof())///checks to see if End Of File... Loops unil end
{
std::string tempString;
std::getline(Splash, tempString);///Takes text from document and stores in tempString

Lines += tempString;///
}
std::cout << Lines << std::endl << std::endl;
}
else
{
std::cout <<"ERROR file could not be loaded or does not exist!";
}
}
/************************************************************************/
{
std::string Lines = "";///NULL string
std::ifstream myFile(FileName.c_str());///ifstream used to read from txt

if(myFile.is_open())
{
while(!myFile.eof())///checks to see if End Of File... Loops unil end
{
std::string tempString;
std::getline(myFile, tempString);///Takes text from document and stores in tempString

Lines += tempString;///
}
std::cout << Lines << std::endl << std::endl;
}
else
{
std::cout <<"ERROR file could not be loaded or does not exist!";
}
}
/************************************************************************/
void Initialize( int argc, char** argv )
{
std::cout << message << std::flush;

/// spin some cycles to pretend that the program is loading resources, etc.
time_t start = time( &start );
while (difftime( time( NULL ), start ) < 6.5 /**seconds*/){}

std::cout << "\r" << std::string( message.length(), ' ' ) << "\r" << std::flush;
}
/***********************************************************************/
{
int Choice;

std::cout << std::setw(50) <<"Welcome To MY RPG!!!" << std::endl;
std::cout << std::setw(73) <<"Enjoy! Your suggestions and constructive criticism are encouraged!" << std::endl;

std::cout <<"\n\n\n\n\n\n\n\nPlease Select an Option: " << std::endl;
std::cout <<"1: New Game \n2: Load Game(In Development) \n3: Help \n4: Quit\n" << std::endl;
std::cin >> Choice;

/**Fixes infinite loop cause by char input instead of numeric!!!*/
while(std::cin.fail())
{
std::cin.clear();
std::cin.ignore(std::numeric_limits<std::streamsize>::max(),'\n');
Clear();
inputError();
//std::cout << "Bad entry.  Enter a NUMBER: ";
//std::cin >> Choice;
}

switch(Choice)
{
case 1:
break;
case 2:
break;
case 3:
Clear();
Help();
break;
case 4:
std::exit(0);
break;
default:
Clear();
inputError();
}
}
/***********************************************************************/
void Help()
{
int Choice;
std::cout << std::setw(53) <<"Looks like you need help!" << std::endl;
std::cout << std::setw(57) <<"Well, you came to the right place!" << std:: endl;

std::cout <<"\n\n\n\n\n\n\n\nPlease Select an Option: " << std::endl;
std::cout <<"1: Game Synopsis \n2: In-Game Command List \n3: Credits \n4: Back\n" << std::endl;
std::cin >> Choice;

while(std::cin.fail())
{
std::cin.clear();
std::cin.ignore(std::numeric_limits<std::streamsize>::max(),'\n');
Clear();
inputError();
Help();
}

switch(Choice)
{
case 1:
break;
case 2:
Clear();
getch();
Clear();
Help();
break;
case 3:
Clear();
getch();
Clear();
Help();
break;
case 4:
Clear();
break;
default:
Clear();
inputError();
Help();
}
}


• It would be nice to turn those loose functions into one or more classes. Currently your are not using global variables, which is nice, but excessively structured/procedural code like this can easily lead to nasty global state that is hard to maintain and hard to debug. I suggest starting the migration to OOP early. Move the code to a Game class and start from there, breaking it further into other classes to achieve the Single Responsibility goal.

• When you declare function prototypes, keep the parameter names. This is a way of self-documenting the code, so you don't need to comment trivial stuff.

• Those large switch statements are crying to be refactored into functions. Instead of writing something like this:

   case 2:
Clear();
getch();
Clear();
Help();
break;


Turn all that into a helper function:

case OptInGameCommands :
displayInGameCommandList();
break;


Also, the magic numbers, 1, 2, 3, etc would arguably be better as constants that describe the action that is performed for a given input form the player. E.g.:

enum MenuOptions
{
OptGameSynopsis   = 1,
OptInGameCommands = 2,
// etc...
};


   time_t start = time( &start );
while (difftime( time( NULL ), start ) < 6.5 /**seconds*/){}


Use a constant for that time amount:

const double secondsToSpin = 6.5;
time_t start = time( &start );
while (difftime( time( NULL ), start ) < secondsToSpin) { }

• A final word on naming: PascalCase names are popular on Windows programming. I personally think this naming is fine for functions and types, though I don't think it is a good idea also applying it to variables names, such as Line and Splash in your code. I recommend using only camelCase or snake_case for variables to differentiate them from class types and function names. And of course, whichever convention you choose, be consistent with it within the project, that's the important thing, rather then which one to choose.

• Dude you rock! This was really helpful, I have been reading up on OOP for C++ haven't yet got the hang of it. That's the main reason I've stuck to functions and procedural code. I'll be working on converting this to OOP and using Constants/enums etc. as soon as I have corrected the mistakes I've made with this so far. I'll be taking into account everything you and Loki have said. I appreciate it =) Will update the code in the next day or two, as I won't be at my house today and most of tomorrow. – TDubs95 Dec 21 '14 at 21:58
• @xChubz, that's great! But post a new question with the revised code instead. Take a look at this discussion. – glampert Dec 21 '14 at 23:01

This is wrong in every language:

    while(!myFile.eof())///checks to see if End Of File... Loops unil end
{


This is because the last successful read read "up to" but not past the EOF. Thus there is no data left but the EOF flag is not set. So condition will allow the loop to be entered evn though the next read is going to fail.

So this line:

        std::getline(myFile, tempString);
tempString += "\n";
Lines += tempString;


Will fail (after all the other lines have been read) and you are not checking for it. Thus Lines has an extra '\n' on the end at the very least. If you had written:

        if (std::getline(myFile, tempString))
{
tempString += "\n";
Lines += tempString;
}


Then Line would have at least bee correct.

But the better way to write this loop is:

    std::string tempString;
while(std::getline(myFile, tempString))
{
Lines += (tempString + '\n')
}
std::cout << Lines << std::endl << std::endl;


SplashScreen();///display Splash screen


I can guess from the name of the function that the splashScreen is going to be displayed. I don't need the extra clutter of this comment. Comments should be reserved for telling my why something is happening. I can easily read in the code how it is happening (unless there is some big magic formula you may want to explain what that is doing).

Comments that repeat what the code is saying are actually WORSE than no comments. This is because over time the comments will fall out of step with the code. You need to take extra care to maintain the comments. Comments that are out of sync with the code are a real problem as the maintainer will come along and see that the code and comments do not match and will have to spend time trying to work out which are correct before fixing one or the other.

Prefer '\n' over std::endl

std::cout <<"Previous selection was invalid, try another from the list: " << std::endl;

// Just write
std::cout <<"Previous selection was invalid, try another from the list: \n";


The difference is an extra flush. The flush will not help you but it can definitely make the performance of your code much worse.

• Thanks for the info, really does help. My post was edited by someone else probably a moderator or something, I had said that I used all the comments for me. I didn't plan on posting the code it was me trying to make sure I knew what I was putting sorry. So yes the comments are terrible hahaha. Thanks for some brutally honest feedback I'll make the changes and neat it up as soon as I get home. I AM A NOOB so all things are helpful. – TDubs95 Dec 21 '14 at 16:53