Text based game challenge

  • (6 Pages)
  • +
  • « First
  • 4
  • 5
  • 6

87 Replies - 36874 Views - Last Post: 24 October 2011 - 06:43 AM

#76 saimanoj  Icon User is offline

  • New D.I.C Head

Reputation: 1
  • View blog
  • Posts: 14
  • Joined: 19-August 10

Re: Text based game challenge

Posted 20 October 2011 - 06:02 AM

Thanks for your comments @ishkabible and @jim.
They are very helpful. As you have guessed i am a Java Programmer.
I started learning C++ recently so thought of giving this challenge a try. It helped me a lot.
Unfortunately I made a major structural change in my code 1 or 2 days before the submission and all the problem aroused after that. I will soon put the previous version of the code. Please check it and tell me how it is.
Was This Post Helpful? 0
  • +
  • -

#77 saimanoj  Icon User is offline

  • New D.I.C Head

Reputation: 1
  • View blog
  • Posts: 14
  • Joined: 19-August 10

Re: Text based game challenge

Posted 20 October 2011 - 07:18 AM

Here is my old version of WordPlay which does not use inheritance in a useless way.
WordPlay previous version
Was This Post Helpful? 0
  • +
  • -

#78 jimblumberg  Icon User is online

  • member icon


Reputation: 3845
  • View blog
  • Posts: 11,737
  • Joined: 25-December 09

Re: Text based game challenge

Posted 20 October 2011 - 08:59 AM

To me this looks much better than your previous solution. However there are still a few problems.

First there is still the possibility of calling srand() more than once. You should be calling srand() once and only once, usually early in the main() function.

Why are you using a ifsream* instead of just an ifstream? You do not need the pointer, you are only using a single instance of the fstream. Since you used new you also need to use delete to release the memory. I would suggest that once you read the file into your vector you close the file right away.

You are still calling exit(), which will not call the class destructors properly. You have this in a member function so return a value to the calling function informing it of the problem, or throw an exception. Most of the code in the initialize() function could be in the constructor.

Your shutdown() function should actually be the class destructor so that it gets called automatically when the class goes out of scope.

In your game play function you have this line:
int length = secretword.length() - 1;

You do not need or want the - 1. If you do the subtraction then you are missing the last character in your word.

Jim
Was This Post Helpful? 1
  • +
  • -

#79 ishkabible  Icon User is offline

  • spelling expret
  • member icon





Reputation: 1616
  • View blog
  • Posts: 5,707
  • Joined: 03-August 09

Re: Text based game challenge

Posted 21 October 2011 - 06:44 PM

ok Hulla, i didn't address everything but i offered my thoughts in the comments and re-wrote many portions of your code to show you how you might better do it. a total re-write of the code(with my comments in mind) will yield much better result!!

major things:

magic numbers - they are evil and force your to skim though tons of code just to change 1 little thing. they are a maintenance nightmare!! enums can fix many of your issue. also some constants to denote what full health, full food is, ect... would be beneficial.

using time as a random number generator. it's not a random number generator use rand and srand for that.

you need to brake your code up into more parts, use structures or class, ect.. make it more moulder

//#include "headers.h" //this dosn't need to be here at all, just include what you need for the file
#include <iostream>
#include <cstdlib>
#include <fstream>
#include <string>
#include <ctime>
//you don't need this at all, avoid using system dependent headers
//#include <windows.h>

//Function prototypes
//prototypes are good for non-static functions,
//however i can already see your abusing refrencing passing being abused
void getEnter(int);
//void getData(int&, int&, int&); this seems like a class or structure would be better
//void saveData(int, int, int); looks like a method for that class
void findCreature(void);
//bool getCommand(int&, int&, int&); another function that could be a method
void instructions(void);

//a structure to store game data
//would be best to have this in a header file
class GameState {
private:
	int health;
	int creaturesSurvived;
	int food;
	std::string filepath;
public:
	GameState(std::string fp) : health(0), creaturesSurvived(0), food(0), filepath(fp) {}
	void getData(); //these fucntions are larger and will not inline well so i define them outside the
	void saveData();
	bool getCommand();
	//these smaller functions do inline well so i define them in the class
	int getCreaturesSurvived() {
		return creaturesSurvived;
	}
	int getHealth() {
		return health;
	}
	int getFood() {
		return food;
	}
	//for incrementing creaturesSurvived
	void incCreaturesSurvived() {
		++creaturesSurvived;
	}
};

//really not a good idea to bring a whole namespace into global scope
//it defeats the purpose of the namespace
//using namespace std;

int main(void) {
    GameState game("Text_Game_2.txtgm"); //ok place to put this, also, i don't like spaces in my filenames
    game.getData();
    instructions();
    if(game.getCreaturesSurvived() == 0) { //ok use of magic number, 1s and 0s are ok if they plainly deal with numbers
        getEnter(1);  // Magic number!! this 1 is not ok as it specifiys a command, an enum would be better
        std::cout << "You start roaming a nearby forest, with a sharp butcher's knife . . .\n";
    } else {
        std::cout << "The game will resume from the last save.\n";
        getEnter(0); // Magic number!! this 0 is not ok as it specifiys a command, an enum would be better
        std::cout << "You continue roaming the forest.\n";
    }
    while(true) {
        findCreature();
        std::cout << "You currently have " << game.getHealth() << "/10 health points and have survived " << game.getCreaturesSurvived() << " creature(s).\n";
        std::cout << "You have " << game.getFood() << " pieces of food left.\n";
        //bool exitOrNot = game.getCommand(); not need for this tempray varible
        if(game.getCommand()) {
        	return 0;
        }
        std::cout << "You continue roaming the forest.\n";
        game.incCreaturesSurvived();
    }
}

void GameState::saveData() {
    std::ofstream outFile;
    outFile.open(filepath.c_str());
    //use constants for newlines '\n'
    //cout is unbufferd so no flushing is required for it
    //for a file like this you should take advantage of buffering by not flushing constently
    //std::endl only has a few good uses actully
    outFile << health << '\n' << creaturesSurvived << '\n' << food << '\n' << "Do Not Modify This File.";
    outFile.close();
}

void GameState::getData() {
	//not a perfect idea to create a file handle on every call
	//would be better to maintain an iostream as a member of the class
	std::ifstream inFile;
	inFile.open(filepath.c_str()); //bad to use constants, i added a filepath member to solve that
	if(inFile.fail()) { //you don't have to test it equals true, just use it as the expresion
		std::cout << "The saved-data has been corrupted and/or removed." << '\n';
		std::cout << "The player's statistics will be reset."            << '\n';
		health = 10; // Magic number!!
		creaturesSurvived = 0; // Magic number!!
		food = 3; // Magic number!!
	} else {
		// Assign health and creaturesSurvived a value that they can never have to check if the data retrieved is valid
		health = 11, creaturesSurvived = -1, food = -1; // Magic number!!
		inFile >> health >> creaturesSurvived >> food;
		// If the health and creaturesSurvived could not be retrieved for whatever reason, assign them 10 and 0 respectively
		if(health == 11 || creaturesSurvived == -1 || food == -1) { // Magic number!!
			std::cout << "The saved-data has been corrupted and/or removed." << '\n';
			std::cout << "The player's statistics will be reset."            << '\n';
			health = 10; // Magic number!!
			creaturesSurvived = 0; // Magic number!!
			food = 3; // Magic number!!
		}
		else {
			std::cout << "Retrieval of saved data was successful." << '\n';
		}
		std::cout << '\n';
	}
}

bool GameState::getCommand() { //this function is bloated, very bloated
    static int exitPermission = false; //iffy use of static varible
    //thse instructions could go in a menu method of sorts
    std::cout << "Enter the number for the command you would like to do.\n";
    std::cout << "(1) Attack it\n";
    std::cout << "(2) Flee\n";
    std::cout << "(3) Replenish health\n";
    std::cout << "(4) Save game\n";
    std::cout << "(5) Exit game\n";
    int command;
    std::cin >> command;
    //1) each of these cases could be it's own function
    //2) there really was no need for the {} on each case
    //3) your use of exitPermission is very confusing
    switch(command) {
	case 1:  // Magic number!!
		health -= (time(0)%10+1);  // Magic numbers!! what dose that even do? learn to use srand and rand
		if(health <= 0) { //magic number but it's ok here
			std::cout << "You were killed.\n";
			exitPermission = true;
			getEnter(2); // Magic number!!
			saveData();  // Magic number!!
			return true;
		}
		std::cout << "You kill the creature with " << health << " health point(s) remaining.\n";
		getEnter(0); // Magic number!!
		return false;
	case 2: // Magic number!!
		std::cout << "Fleeing . . .\n";
		//Sleep((time(0)%2+1)*1000); //bleh, what is this for?
		if(time(0)%3 == 0) { // Magic number!!
			std::cout << "You successfully escape!\n";
			getEnter(0); // Magic number!!
			return false;
		}
		std::cout << "You were killed while you were fleeing.\n";
		getEnter(2); // Magic number!!
		exitPermission = true;
		return true;
	case 3: // Magic number!!
		if(food == 0) {//this is a magic number but it's *ok*. 1s and 0s(somtimes 2's) are somtimes ok when used as simple numbers
			std::cout << "You do not have any more food to heal with!\n";
			getCommand();
			if(exitPermission) return true;
			return false;
		}
		std::cout << "You eat a piece of food, and heal all your health-points.\n";
		food--;
		health = 10; // Magic number!!
		getCommand();
		if(exitPermission) return true;
		return false;
	case 4: // Magic number!!
		saveData();
		std::cout << "Game saved.\n";
		getEnter(0); // Magic number!!
		getCommand();
		if(!exitPermission) return false;
		return true;
	case 5: // Magic number!!
		exitPermission = true;
		getEnter(2); // Magic number!!
		return true;
	default:
		std::cout << "Unrecognized command.\n";
		getCommand(); //iffy recusive call, using a loop might be better
		if(!exitPermission) {
			return false;
		}
		return true;
    }
    return false;
}

void instructions(void) {
    std::cout << "Would you like see the instructions? (\"Y\"/\"N\")\n";
    char seeInstructions = 'G';
    do {
        std::cin >> seeInstructions;
        if(seeInstructions == 'n' || seeInstructions == 'N') return;
        else if(seeInstructions != 'y' && seeInstructions != 'Y')
        std::cout << "Unrecognized command: \"" << seeInstructions << "\"" << '\n' << "Please try again.\n";
    } while(seeInstructions != 'y' && seeInstructions != 'Y');
	//you don't need or want all those endls, you could actully make this 1 expresion b
    std::cout << "This game relies partially on chance and skill.\n";
    std::cout << "Your outcome in the game is dependant on your actions.\n";
    std::cout << "The objective of this game is to choose what action to do,\n";
    std::cout << "depending on your health and whether or not you feel lucky.\n";
    std::cout << "The actions you can do are flee, attack and replenish your health.\n";
    std::cout << "There is a 1 in 3 chance of fleeing successfully. Food replenishes\n";
    std::cout << "all of your health-points, which are the determination of your vitality.\n";
}

void getEnter(int gameStart) { //you use magic numbers with game start, bleh
	//you had some very strange formating here
	//would have been best to combine enums with a switch case here
	if(gameStart == 1) { //magic number, should have used an enum instead
		std::cout << "Press enter to start your adventure . . .\n";
	} else if(gameStart == 0) { //magic number, should have used an enum instead
		std::cout << "Press enter . . .\n";
	} else { //implicit magic number, should have used an enum instead
		std::cout << "Press enter to exit . . .\n";
	}
    //cin.sync(); //why dose cin need to be synced?
    std::cin.get();
}

void findCreature(void) {
	//using time as a random number genrator is not the greatest idea
    //int findCreatureTime = (time(0)%3+2)*1000; //why dose this need to be here?
    //Sleep(findCreatureTime); //why dose this need to be here?
    std::cout << "You encounter a strange creature!\n";
}


This post has been edited by ishkabible: 21 October 2011 - 06:45 PM

Was This Post Helpful? 0
  • +
  • -

#80 hulla  Icon User is offline

  • sqrt(-1) am awesome


Reputation: 49
  • View blog
  • Posts: 727
  • Joined: 05-March 11

Re: Text based game challenge

Posted 22 October 2011 - 01:23 AM

I need windows.h for Sleep() which makes the wait-time for a monster random.

Is it the best option to use std:: before all the standard functions or to write something like using std::/*Whatever*/; or simply use using namespace std;?

Hold on I'll review the code soon. I need to go.

Darn it the 4th inline transformed ( : / ) without the spaces to an emoticon.
Was This Post Helpful? 0
  • +
  • -

#81 jimblumberg  Icon User is online

  • member icon


Reputation: 3845
  • View blog
  • Posts: 11,737
  • Joined: 25-December 09

Re: Text based game challenge

Posted 22 October 2011 - 07:56 AM

@ishkabible
In you comments of the above code you state:

Quote

   //use constants for newlines '\n'
    //cout is unbufferd so no flushing is required for it


First, cout is buffered.

Second when dealing with input and output from the console you will never notice the difference between using endl or '\n'. However when dealing with file input and output the over use of endl can be problematic. If you want to insure your output is written to your file reliably then you may want to force the write with either fflush() or endl instead of waiting until the buffer fills or the closing of the file, which also flushes the buffer. If you wait until the buffer is filled and you have a program crash then the data may or may not be written to the file correctly. I do recommend that you use the flushing of the stream (endl, fflush()) sparingly during file input and output to reduce access times.

Jim
Was This Post Helpful? 1
  • +
  • -

#82 ishkabible  Icon User is offline

  • spelling expret
  • member icon





Reputation: 1616
  • View blog
  • Posts: 5,707
  • Joined: 03-August 09

Re: Text based game challenge

Posted 22 October 2011 - 10:49 AM

Quote

First, cout is buffered.


jim, i thought cout wasn't buffered becuase when you output anything just using '<<' without flushing it still outputs to console. if it was buffered then wouldn't it only output on overflow or a flush? i can't seem to find anything that says weather or not it's buffered :/ stdout is always buffered so it would follow that cout is buffered. now i have a question, why do i not see the effects of buffering?

Quote

Second when dealing with input and output from the console you will never notice the difference between using endl or '\n'


exactly, so why flush it when the 2 act the same way?

This post has been edited by ishkabible: 22 October 2011 - 11:31 AM

Was This Post Helpful? 0
  • +
  • -

#83 jimblumberg  Icon User is online

  • member icon


Reputation: 3845
  • View blog
  • Posts: 11,737
  • Joined: 25-December 09

Re: Text based game challenge

Posted 22 October 2011 - 11:53 AM

Quote

i thought cout wasn't buffered becuase when you output anything just using '<<' without flushing it still outputs to console.

Of the four standard streams cin, cout, cerr, and clog only cerr is unbuffered. See this link.

Actually it is possible, but difficult, to write something to cout and not have it show up. Try something like:
#include <iostream>
#include <cstdio>

int main()
{
   std::ios::sync_with_stdio(false);
   std::cout << "Hello" << std::endl;
   std::cout << "World";
   getchar();
}




You should see a pause between the "Hello" and the "World" until you enter a character for the getchar(). When a program ends all stream buffers are flushed before the program ends. The sync_with_stdio() un-links the C input output streams from the C++ streams. Also if you use an input operation, this will cause a output stream flush by default.

My point was using '\n' or endl are basically the same when using console input and output, the user will not notice any difference, because the user is much slower processing this information than the computer. So which method you use is an individual choice.

EDIT:
Also the '\n' and endl do not act the same. The '\n' just puts the newline character into the stream. The endl puts the newline character into the stream then flushes the buffer.

Jim

This post has been edited by jimblumberg: 22 October 2011 - 12:13 PM

Was This Post Helpful? 2
  • +
  • -

#84 ishkabible  Icon User is offline

  • spelling expret
  • member icon





Reputation: 1616
  • View blog
  • Posts: 5,707
  • Joined: 03-August 09

Re: Text based game challenge

Posted 22 October 2011 - 05:06 PM

View Postjimblumberg, on 22 October 2011 - 07:53 PM, said:

Actually it is possible, but difficult, to write something to cout and not have it show up. Try something like:
#include <iostream>
#include <cstdio>

int main()
{
   std::ios::sync_with_stdio(false);
   std::cout << "Hello" << std::endl;
   std::cout << "World";
   getchar();
}




interesting, what makes it so difficult to show the effects of buffering? if i replace getchar() with cin.get() it doesn't happen.
Was This Post Helpful? 0
  • +
  • -

#85 jimblumberg  Icon User is online

  • member icon


Reputation: 3845
  • View blog
  • Posts: 11,737
  • Joined: 25-December 09

Re: Text based game challenge

Posted 22 October 2011 - 05:25 PM

When you use cin the system automatically flushes the output buffer.

See this link on cin. And from that link:

Quote

cin is tied (see ios::tie) to the standard output stream cout, which means that cout's buffer is flushed (see ostream::flush) before each i/o operation performed on cin.


Jim
Was This Post Helpful? 2
  • +
  • -

#86 ishkabible  Icon User is offline

  • spelling expret
  • member icon





Reputation: 1616
  • View blog
  • Posts: 5,707
  • Joined: 03-August 09

Re: Text based game challenge

Posted 22 October 2011 - 05:37 PM

which is why un-syncing cin and cout with stdio allows you to use just use stdin(e.g. getchar()) and not have cout be flushed by cin. i never realized that, thanks jim!
Was This Post Helpful? 0
  • +
  • -

#87 saimanoj  Icon User is offline

  • New D.I.C Head

Reputation: 1
  • View blog
  • Posts: 14
  • Joined: 19-August 10

Re: Text based game challenge

Posted 24 October 2011 - 06:12 AM

View Postjimblumberg, on 20 October 2011 - 08:59 AM, said:

To me this looks much better than your previous solution. However there are still a few problems.

First there is still the possibility of calling srand() more than once. You should be calling srand() once and only once, usually early in the main() function.

Why are you using a ifsream* instead of just an ifstream? You do not need the pointer, you are only using a single instance of the fstream. Since you used new you also need to use delete to release the memory. I would suggest that once you read the file into your vector you close the file right away.

You are still calling exit(), which will not call the class destructors properly. You have this in a member function so return a value to the calling function informing it of the problem, or throw an exception. Most of the code in the initialize() function could be in the constructor.

Your shutdown() function should actually be the class destructor so that it gets called automatically when the class goes out of scope.

In your game play function you have this line:
int length = secretword.length() - 1;

You do not need or want the - 1. If you do the subtraction then you are missing the last character in your word.

Jim


Thanks for your feedback again.
Actually, the secretword.length() - 1 is a mistake i identified and cleared but as i posted an earlier version there are newer mistakes.

as far as ifstream thing is concerned, i used ifstream* because i wanted to create the ifstream class object in the class and use it in different methods. I am unable to do it with simple object. So, i had to create a pointer. That is the only reason i used pointer for ifstream.

Also I dont know much about calling exit(). I used to use it very much in C language to exit the whole program when it should not go further.

Now, I have separated the srand from randomize() function and placed in initialize() function.

I will make the changes specified by you and will be back soon.
Was This Post Helpful? 0
  • +
  • -

#88 jimblumberg  Icon User is online

  • member icon


Reputation: 3845
  • View blog
  • Posts: 11,737
  • Joined: 25-December 09

Re: Text based game challenge

Posted 24 October 2011 - 06:43 AM

Quote

as far as ifstream thing is concerned, i used ifstream* because i wanted to create the ifstream class object in the class and use it in different methods. I am unable to do it with simple object. So, i had to create a pointer. That is the only reason i used pointer for ifstream.


In the code you provided there is no reason for the ifstream pointer. You create the instance of this class with ifstream yourStream; in you class declaration, then in your implementation you just open a file using
yourStream.open("WHATEVER.FILE");.

Quote

Also I dont know much about calling exit(). I used to use it very much in C language to exit the whole program when it should not go further.

There is nothing wrong with using exit() in a C program. But since this is a standard C function, it knows nothing about C++ classes and therefore it does not call your C++ destructors properly.

Quote

Now, I have separated the srand from randomize() function and placed in initialize() function.

Unless you insure that this class is a "singleton class" srand() should not be placed inside the class. This function uses a static memory variable to hold the "seed" and multiple calls to srand() overwrite this seed. That is why it should only be called once and only once.


Jim
Was This Post Helpful? 0
  • +
  • -

  • (6 Pages)
  • +
  • « First
  • 4
  • 5
  • 6