Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am not really having any problems with this. I'm just wondering if anybody has any ideas for a path for me to look down for bettering this little FF turn-based style of game in my free time.

I know somebody is going to complain about my global variables and the fact that I am not using object-oriented programming, but it will eventually get there. This is a boredom project that I'm plunking through.

Any advice or insight on any issues or ideas would be cool. And if somebody is looking into making a turn-based style of game, they can feel free to use up my code. Sorry about any bad formatting as well; I haven't really learned that yet.

// random.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include<iostream>
#include<cstdlib>
#include<cmath>
#include<ctime>

using namespace std;
int main()
{

        int eHealth=100;  //initial Enemy Health(based on enemy)
        int pHealth=100;  //initial Player Health 
        int eAttack=0;    //Enemy Attack Strength(based on enemy)
        int pAttack=0;    //Player Attack Strength(based on level and items)
        int selection=0;  //Selection Variable for Battle Menue
        int hPower=0;     //Healing power Variable(random number)
        int eSelection=0; //Enemies battle menu selection variable
        int counter=0;    //Counter to establish whos turn it is 
        int itemSelect=0; //Item inventory selection
        int eMagic=50;    //Enemies magic meter
        int pMagic=50;    //Players magic meter


        cout<<pHealth<<"        "<<eHealth<<endl;  //player and enemy health levels
        cout<<pMagic<<"        "<<eMagic<<endl;    //player and enemy magic levels
        cout<<endl;
        cout<<endl;
        cout<<"1.Attack"<<endl;                     //lines 31-33 are the battle menu appears frequently throught game
        cout<<"2.Heal"<<endl;
        cout<<"3.Item"<<endl;
    do                  //start of post test loop that runs the game until somebody is dead
     {  

            if(counter==0)// if the counter variable is 0 it is the players turn
            {
             cin>>selection;
             srand(static_cast<int>(time(0)));//randomize all the random variables

               switch(selection)
               {        
                case 1:// player chooses to ATTACK
                  pAttack=1+rand()%(35-1+1);//attack power can be between 1-35
                  cout<<"ATTACK "<<pAttack<<endl;
                  system("pause");
                  system("cls");
                  eHealth=eHealth-pAttack;
                  cout<<pHealth<<"        "<<eHealth<<endl;
                  cout<<pMagic<<"        "<<eMagic<<endl;
                  cout<<endl;
                  cout<<endl;
                  cout<<"1.Attack"<<endl;
                  cout<<"2.Heal"<<endl;
                  cout<<"3.Item"<<endl;
                  break;
                case 2://Player chooses to heal, it costs 10 magic to heal so if you dont have enough magic you loose a turn
                  if(pMagic>9)
                  {
                   hPower=1+rand()%(35-1+1);//healing power can be any number between 1-35
                   cout<<"HEAL"<<hPower<<endl;
                   system("pause");
                   system("cls");
                   pHealth=pHealth+hPower;
                   pMagic=pMagic-10;
                   cout<<pHealth<<"        "<<eHealth<<endl;
                   cout<<pMagic<<"        "<<eMagic<<endl;
                   cout<<endl;
                   cout<<endl;
                   cout<<"1.Attack"<<endl;
                   cout<<"2.Heal"<<endl;
                   cout<<"3.Item"<<endl;
                   }//endIF
                break;

                case 3:// if player chooses to use item inventory system
                cout<<"ITEM INVENTORY"<<endl;
                cout<<"1.Potion            Restores HP"<<endl;
                cout<<"2.Ether             Restores MP"<<endl;
                cout<<"3.Bomb              Causes Damage to all players"<<endl;
                cout<<"4.Big Purple Dildo  ?????"<<endl;
                cout<<"5.Sticky Bomb       Freezes oponent for 3 Truns"<<endl;

                cin>>itemSelect;
                    switch(itemSelect)
                    {
                        case 1:
                            pHealth=pHealth+75;
                            system("cls");
                            cout<<pHealth<<"        "<<eHealth<<endl;
                            cout<<pMagic<<"        "<<eMagic<<endl;
                            cout<<endl;
                            cout<<endl;
                            cout<<"1.Attack"<<endl;
                            cout<<"2.Heal"<<endl;
                            cout<<"3.Item"<<endl;
                            break;
                        case 2:


                            break;
                    }//endItemSwitch        
            }//endSelectionSwitch
        counter=1;// advances the counter to 1 to allow the enemies turn
    }//endif
            eSelection=rand() % 2+1;
                switch(eSelection)
                {
                    case 1:
                    eAttack=1+rand()%(35-1+1);
                    cout<<"ATTACK "<<eAttack<<endl;
                    system("pause");
                    system("cls");
                    pHealth=pHealth-eAttack;
                    cout<<pHealth<<"        "<<eHealth<<endl;
                    cout<<pMagic<<"        "<<eMagic<<endl;
                    cout<<endl;
                    cout<<endl;
                    cout<<"1.Attack"<<endl;
                    cout<<"2.Heal"<<endl;
                    cout<<"3.Item"<<endl;
                break;

                case 2:

                    if(eMagic>10)
                    {
                       hPower=1+rand()%(35-1+1);
                       cout<<"HEAL"<<hPower<<endl;
                       system("pause");
                       system("cls");
                       eMagic=eMagic-10;
                       eHealth=eHealth+hPower;
                       cout<<pHealth<<"        "<<eHealth<<endl;
                       cout<<pMagic<<"        "<<eMagic<<endl;
                       cout<<endl;
                       cout<<endl;
                       cout<<"1.Attack"<<endl;
                       cout<<"2.Heal"<<endl;
                       cout<<"3.Item"<<endl;
                    }
                break;
            }//endeSelectionSwitch
                counter=0;
    }while(eHealth > 1 && pHealth > 1);//loops while both players life is over 1 ends postest loop when players life or enemies falls below 1

    system("pause");
    return 0;
    }
share|improve this question
4  
Learn how to write a function in C++! –  Apprentice Queue Jan 3 at 23:14
    
Havent gotten that far yet but I assume that turning my menu into a function would clean this up quite a bit? –  John Snow Jan 3 at 23:16
1  
Those variables aren't actually global; they're just all at the top of main(). –  Jamal Jan 3 at 23:17
add comment

3 Answers

  • system("PAUSE") is entirely unnecessary here. You could do the same thing by simply clearing out cin and then reading a char.

  • You should generally call srand once, at the beginning of the program. Once you've seeded the PRNG, it'll be good for a while -- probably much longer than this program will run. :) The way you're doing it, if the loop runs twice in the same second, and the user makes the same choices, the same outcome will occur.

  • You are doing an awful lot of similar I/O over and over -- particularly, outputting a menu and getting a response. You should look at creating a function to do that, and then just call it whenever you want the user to choose something.

  • using namespace std; is laziness. :) If you don't want to type std:: before that stuff, i'd recommend at least only using the names you actually use. Otherwise, you could run into interesting issues later when you decide to use the same names that the standard library used (which is not at all uncommon).

  • You have a bunch of magic numbers. You'd do better to make those constants; then you could use them by name, and wouldn't need comments telling people what they mean. :)

share|improve this answer
    
Thats what I figured I havent written functions yet just simple I/O stuff. I just learned about the rand function today so that was the inspiration that got me playing around. Im in between programming semesters so Im bored. –  John Snow Jan 3 at 23:20
    
@cHao: Obligatory post regarding using namespace std: stackoverflow.com/questions/1452721/… –  Jamal Jan 3 at 23:23
    
Did not know tht was bad practice thats the way C++ was broken down to me... Thanks for the link Jamal. Makes sense –  John Snow Jan 3 at 23:29
    
@JohnSnow: Also see codereview.stackexchange.com/questions/27986/… (my advice to someone who was working on a project a bit like yours). Includes a bit about the benefits of OOP near the end. –  cHao Jan 3 at 23:41
add comment

Your code is repetitive; it looks like you are trying to "unroll" the entire storyline. Ideally, you should write each chunk of code just once. Figure out where the storyline rejoins itself — draw a flowchart. Then write a function for each action (e.g. attack).

Before you try to write the whole game, start small. Try to implement simple primitives first.

You need to be able to pass values to a function, and using global variables would be a bad idea. Instead, group related variables into a struct:

struct Character {
    int health;
    int attack;
    int magic;
};

void heal(Character &c) {
    int healingPower = 1 + rand() % 35;
    c.magic -= 10;
    c.health += healingPower;
}

void print(const Character &c, std::ostream &out) {
    // TODO
}

int main() {
    Character self, enemy;

    print(self, std::cout);
    heal(self);
    print(self, std::cout);
}

Verify that the healing process works. Debug that, and build from there!

share|improve this answer
add comment
  • Do not call std::srand() repeatedly in a program. This will reset the seed at each call, causing std::rand() to produce the same random number. It's best to put this at the top of main() where it'll be called only once. It's also easier to maintain this way.

    Also prefer to call it like this (assuming you receive "loss of data" warnings from not having a cast):

    std::srand(static_cast<unsigned int>(std::time(NULL)));
    

    If you're using C++11, use nullptr instead of NULL.

  • Prefer to use "\n" when outputting a newline without a buffer flush. std::endl does both, so using it quite often could slow down your program.

    Here's an example of how it's used:

    std::cout << "\nthis message contains two newlines\n";
    
share|improve this answer
    
Awesome tip fixed a problem that was in the back of my head every time I ran the program. –  John Snow Jan 3 at 23:34
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.