Not very efficient

[cob]

Hey all! As stated many times, I'm working on this rpg from this tutorial blah blah blah. But, I've hit a snag with the items system so, I went back and looked at what I did and, it's very inefficient.

Basically, it's an if-else-if ladder but, as you can see, that code gets reused a LOT. There are goign to be 23 rooms plus two hidden rooms. Please to helps!

#include <cstdlib>
#include <iostream>
#include <cstring>
#include <string>
using namespace std;

bool running = 1; //Allows for easy termination of the program
void titlefunc(); //Handles the title screen
void newgame(); //starts a new game
int userInput = 0; //will be used to get data from the player
int playerloc = 0; //will store a numerical reference to the player's location
int playerinfo[4]; //will store player data (glass, gender, etc)
//int playeritems[maxItems][10]; //creates the array of MaxItems values with 10 slots
                               //to store item qualities as data
const int maxItems = 100; //Inventory capacity = 100
//string itemNames[];
short playeritemcount = 0; //tracks how many items the player has

int main(int argc, char *argv[])

{

    titlefunc();
    while(running)  //Will continue to loop while running is 1/true
    {
        if(playerloc ==1)
        {
           cout << "You're in a stone room, torches lit around you.\n";
           cout << "There are exits to the east and west.\n";
           cout << "1. Go West\n2. Go East\n";
           cin >> userInput;
           cout <<endl <<endl;

           if(userInput == 1) playerloc = 2;  //West was chosen
           else if(userInput == 2) playerloc = 14; //East was chosen
        }
       //...
            else if(playerloc == 14)
            {
                cout <<"There is an enemy in this room!\n";
                //code for enemy/battle
                cout << "There is only the door through which you came "
                     << "and one to the North. There are torches on the walls "
                     << "and the body of your slain enemy on the floor.\n";
                cout << "1. Go North\n2. Go South\n";
                cin >> userInput;
                cout <<endl<<endl;

                if(userInput == 1) playerloc = 16;
                else if(userInput == 2) playerloc = 13;
            }
        
    }



    system("PAUSE");
    return EXIT_SUCCESS;
}


void titlefunc()
{
    cout << "\t\t\t\t----Dungeon----\n\n\n"; //displays the title
    cout << "\t\t\t\t    1. Play\n"; //gives the option to play the game

    cin >> userInput;
    cout << endl <<endl;

    if(userInput == 1)
    {
        newgame();
    }
    else
    {
        running = 0; //If the user does not enter a valid command, terminate the program
    }

    return;
}

void newgame()
{
    cout << "Welcome to the Dungeon, a demo RPG!\n\n";
    cout << "Since you are new, hero, I must ask some questions.\n";
    cout << "Choose your gender:\n"
         << "1. Male\n2. Female\n";
    cin >> userInput;
    cout <<'\n';
    playerinfo[0] = userInput; //stores the gender in the first space in the array

    cout << "What is your class?\n";
    cout << "1. Warrior\n2. Ranger\n3. Assassin\n";
    cin >> userInput;
    cout << '\n';
    playerinfo[1] = userInput; //stores the class in the second space in the array

    playerloc = 1; //Place the player at the starting area

   /* playerItems[playerItemCount][0] = 0;
    playerItems[playerItemCount][1] = 1;
    playerItems[playerItemCount][2] = 5;
    playerItemCount++; // The player has recieved an item, so
                       //tell the program to remember that*/


    return;
}

Thank you kindly!

[Dy7SLyTq]

first you have to declare and define max items before you can use it.

[AbstractionAnon]

What do you think is inefficient about your code?
I don't see anything that jumps out at me as being inefficient.

I do have a couple of suggestions though.
You should learn to use enums and structs.

For example, assign enum labels to your rooms.

 enum room_names 
{ WestRoom = 2,
   EastRoom = 14,
   ... etc 
};
...
if (playerloc == EastRoom)

If will make your code much more readable. Obviously, you'll want to use more meaningful names.

As for structs, you don't want to keep different kinds of information in an array. It's a pain to keep track of what's in each slot.

int playerinfo[4]; 

Instead, use a struct:

struct Player
{  int gender;
    int type;
    int health;
... etc
} player;
...
cin >> player.gender; 
...
cin >> player.type;

edit: Corrected use of reserved word.

[JLBorges]

> Basically, it's an if-else-if ladder but, as you can see, that code gets reused a LOT.
> There are goign to be 23 rooms plus two hidden rooms.

Use data-driven approach rather than a code-driven approach.

This snippet is just a rough sketch to give you an idea of what was meant by 'a data-driven approach'

#include <iostream>
#include <string>

enum { NORTH = 0, EAST = 1, SOUTH = 2, WEST = 3 } ;
const std::string dir[4] = { "north", "east", "south", "west" } ;

struct room_info
{
    // location after moves to NORTH, EAST, SOUTH, WEST
    int location_after_move[4] ; // -1 if move is not possible

    std::string description ;
};

const int NUM_ROOMS = 25 ;

const room_info room_details[ NUM_ROOMS ] =
{
    // room 1 - no doors towards N,W; E => 1, S => 5
    { { -1, 1, 5, -1 }, "whatever ..." }, // room 0

    // room 1 - no doors towards N,S; E => 14, W => 2
    { { -1, 14, -1, 2 }, "You're in a stone room, torches lit around you." },

    // ... room 2

    // ... room 3

    // ...

    // room 14 - no doors towards E,W; N => 16, S => 13
    { { 16, -1, 13, -1 }, "There are torches on the wall\nand the body..." }

    // etc..

};

int playerloc = 0; //will store a numerical reference to the player's location
bool running = true ; //Allows for easy termination of the program

int main()
{

    // ....

    while( running )
    {
       const room_info& info = room_details[playerloc] ;

       std::cout << info.description << '\n' ;

       std::cout << "possible moves are: " ;
       for( int i = NORTH ; i <= WEST ; ++i )
           if( info.location_after_move[i] != -1 ) std::cout << dir[i] << ' ' ;

       std::cout << "\n0. go north 1. go east ... ? " ;
       int user_input ;
       std::cin >> user_input ;
       if( user_input<NORTH || user_input>WEST || info.location_after_move[user_input] == -1 )
       {
           // error in input
           // handle it
       }

       playerloc = info.location_after_move[user_input] ;
    }

    // ...
}

[cob]

I thought it seemed inefficient because of the redundancy of it. 20+ if and else if commands plus nested ones inside those frequently. Maybe redundancy isn't as inefficient as I think?

Anyway, both of you used enum and I like the way it works so, that seems like something I should learn.

For now, it's not something I need to worry about but, how can I assign some data to the user's class. So that they can have some different battle stats and such? I don't think I need to worry about it until after I get the battle system made

[AbstractionAnon]

Redundant code is code that is identical. You're not doing that. Each of your if/else trees are unique. Redundant code should be moved to a function.

Inefficienct code usually comes from executing the same thing over and over again unnecessarily.

Look seriously at JL Borges suggestion to use a data driven approach.
You can expand his room_info struct to include objects, enemies, etc.

how can I assign some data to the user's class

Expanding the example I gave you above of a struct containing information about a player.

  cout << "What is your class?\n";
  cout << "1. Warrior\n2. Ranger\n3. Assassin\n";
  cin >> player.type;
  switch (player.type) 
  { case 1: player.health = 75;  // Warrior
                  break;
     case 2: player.health = 50; // Ranger
                  break;
     case 3: player.health = 100;  // Assassin
                  break;
  }

[cob]

Thank you guys so much for this advice!

As for the class. You've used player.health but, I could also use any number of traits, correct? I'd like the standout feature of the game(if such a simple game could have one) to be damage calculation using several statistics such as endurance: controlling the number of times the player is allowed to attack in a row; dexterity: affective the successful attack ratio; and the standards, power, speed(which will be approached with a timer... more on this when I get remotely close to it) and weapon stats.. once i have the item system worked out, each item will have a name, a type, and it's effect... weapons and armor will have trait modifiers and some will have damage modifiers as well.

Anyway, for the class, i can included several statistics in each case and not just one, I'm assuming. is there any place I could use a switch statement that would be more effective than an if-else-if (IE when the player has four or more options?)

The game has also automatically taken me back to the previous menu after talking to a character. Is this something about the way c++ reads code or possibly code::blocks? Is there any code I should include to insure that any interaction inside a room will lead you back to the room menu after the interaction is complete?

It's really the next few steps where I am shaky. I'm getting more interface down but, the "AI" part of it, the inventory system, the battle system, these are the parts I'm having trouble with. I think it's just thoroughly more complex than I thought it would be >_<

[AbstractionAnon]

You've used player.health

That was just an example. You're limited only by your creativity. THe point was to demonstrate using named members of a struct rather than anonymous elements of an int array.

is there any place I could use a switch statement that would be more effective than an if-else-if

Switch statements are certainly easier to read than a long series of if-elses.
They are certainly appropriate where you want to choose different actions based on the value of a single variable. I'd say if you have 3 or more choices, use switch rather than if-else.

The game has also automatically taken me back to the previous menu after talking to a character. Is this something about the way c++ reads code or possibly code::blocks?

Nope. Purely a function of your logic.

One point I wanted to make about JLBorges' data driven approach was that by separating the data from the logic, you can load the table from a data file at the start of the program. By changing the data file, you get a different game, with a different map, enemies in different rooms, objects in different room, different paths between rooms, etc, all by changing the data file. The program doesn't change.

[cob]

That's seriously intense stuff! Any recommended readings on the subject? I think I need to learn more about c++ and restructure the game from the ground up.