Game Hierarchy Problem

[Exispistis]

It was suggested to me yesterday that I re-structure my game's class hierarchy (see post four on the following page): http://cplusplus.com/forum/beginner/68037/

I have since set it so that the "Player" and "Enemy" classes inherit from the "GameObject" class, but a couple of errors have arose. As the compiler stated:
"error: expected class name before '{' token" (error in "Player.h") and "error: 'Enemy' does not name a type" (error in "GameObject.h").
The affected code is below:

Player.h:

#include "GameObject.h"

class Player: public GameObject{
    public:
void setAtk(int a){
atk = a;
}
void setDef(int a){
def = a;
}
void setMag(int a){
mag = a;
}
void setHeal(int a){
heal = a;
}
void setMaxheal(int a){
maxheal = a;
}
void setMon(int a){
mon = a;
}
void setExp(int a){
exp = a;
}
void setSpeed(int a){
speed = a;
}
    protected:
int atk, def, mag, heal, maxheal, mon, exp, speed;
};

Enemy.h:

#include "GameObject.h"
#include "AemRandW.h"
#include "Player.h"

#include <iostream>
#include <cstdlib>
#include <ctime>

using namespace std;


class Enemy: public GameObject
{
public:
    Enemy()
    {
        srand(time(0));
    };
    void setStats(int a, int b, int c, int d, int e, int f, int g);
    void setApproachText();
    void DetermineFightFirst();
    virtual void attack() = 0;
    ~Enemy();
protected:
    int enemyAttack, enemyDefence, enemyMagic, enemyHealth, enemyMaximumHealth, enemyMoney, enemySpeed, damage;
};

GameObject.h:

#include "Player.h"
#include "Enemy.h"

using namespace std;

class GameObject
{
    public:
        GameObject();
        int FightDamage(bool EnemyDealing);
        ~GameObject();
    protected:
    int level;
    private:
    Enemy Eno;
    Player Plo;
};

What do I have to do to resolve these errors?

[R0mai]

You have circular dependencies.
GameObject has a member of type Player, and Player inherits from GameObject (so GameObject has a member of type GameObject because Player is a GameObject).
Apart from the fact that you don't have inclusion gurads (or you didn't paste it here) you still need to redesing your class hierarchy.

[MrHutch]

I'm not sure the set up is right here.

I don't think you want your GameObject class to contain instances of Players or Enemies.

I reckon if you take them out (also removed the header includes) you should be fine.

Personally, I think the inheritance needs rejigged. Don't forget that one of the major points of inheritance is that the subclass as the attributes of the superclass. Both players and enemies have things like health and magic so it'd probably be a good idea to inherit those from a superclass.

Whenever I do this sort of thing, I normally go for something like this (you'll have to excuse how trimmed down these are, I've done them quickly for example purposes):

class GameObject
{
   public:
      void Draw(); // Function that draws objects to screen
                   // Probably the same across most objects
   protected:
      int x, y;    // Position coords (I'd usually use vectors but it's for e.g.)
      // Probably more stuff here like a pointer to the sprite, that sort of stuff
};

class LivingObject: public GameObject  // Think of a better name than LivingObject!
{
   public:
      void SetHealth(int);  
   protected:
      int health, magic, att, def;
};

class Player: public LivingObject
{
   // Player specific stuff
};

class Enemy: public LivingObject
{
   // Enemy specific stuff
};

The main point there being that there's a lot that Player and Enemy can share, so the could inherit from a common class. Saves a bit of rewriting.

[Exispistis]

@R0mai I was wondering if the problem was circular dependencies. Thanks for specifying that, though. Also, there are header guards (Code::Blocks automatically generates them), I just didn't copy and paste them to the previous post.

@iHutch105 I had never considered a living objects class, but a very good idea. One question though: how would I use a function in the "LivingObject" class to compare the stats (i.e. compare the speed) of the player and the enemy? Would such a function be more appropriately placed in another part of the program than LivingObject?

[MrHutch]

If you want to compare the stats of each, you can just use something like a 'getter' function.

// In LivingObject (or a better named alternative)
int GetSpeed() const
{
    return speed;
}

// Whenever comparing them
if (player.GetSpeed() > enemy.GetSpeed())
{
   // Player is faster
}

You could make a function where you pass in two LivingObjects and return the fastest if you prefer.

[Exispistis]

Good idea. Thanks! One more question: to determine how much damage is done when the player attacks the enemy (and vice versa), a couple of lines of code will have to be executed to determine how much damage is done (that should look something like this):

damage = rand()%Enemy.atk - rand()%Player.def;
if(damage<0){damage=0;}

The problem is: I am not sure of the most efficient way to make the program figure out what enemy it is talking about. Do I use a switch statement to cover every enemy on the map (represented by different numbers), or is there a more efficient way?

[MrHutch]

How about something like a TakeDamage() function?

// Again, probably in LivingObject
void TakeDamage(int damage)
{
   this->damage-=damage;
}

// Then when you've worked out the damage to take off, call it on the object being damaged
player.TakeDamage(damage);

As for your second question, it's a little hard to answer without understanding how the map works (or the game, in fact - is it turn based? Graphical? Is damage inflicted when a collision is detected). Is it tile-based? Coordinate-based?

I'm heading home from work now, so I'll probably not be able to respond for a couple of hours.

If you're still stuck or someone else hasn't helped out, I'll be happy to take another look then.

[Disch]

Had I noticed this thread I would have replied here, heh.

I already made a big post about some design ideas in the other thread. I don't know if it's exactly on point with what you're asking here though, since I didn't really read this thread (gotta go to work!)

http://cplusplus.com/forum/beginner/68037/#msg363093

[Exispistis]

@Disch Thanks, this was helpful!

@iHutch105 Thanks! Sorry for my lack of specification. It is a turn-based, text-based game with the player's position being stored as coordinates (named "locx" and "locy" respectively). The map is stored in a dynamically allocated 2D int array, with each number being a different thing (couldn't help but to state the obvious)! For example, 0 = unusable tile, 1 = path, 2 = start location (turned to path as soon as the program has used this to determine starting coordinates), 3 = exit, 4 = goblin etc.. Each time the turn is taken, the relevant array element is read, which is then checked against a switch statement. The relevant processes then occur, before the player takes his next turn.

Would I be best to have it so that the function is called for the relevant class by the switch statement for the map? Or is there a better way?

[MrHutch]

So the player will fight a goblin when he's in the same coordinates?

In that case, getting the right goblin would just be a case of looping through the goblins to see which coordinates match that of the player. Something like a GetPosition() function, maybe. Looping through them all isn't greatly efficient, I know, but as long as it's not a huge game it shouldn't be too much of an overhead, especially for a console application.

If it's not too much rework, it might be a better idea to have a tile class for the map. You can store more information that way.

Something like:

class Tile
{
private:
   int x, y; // Position coords.  
public:
   Init(int x, int y);
   bool HasEnemy();  // Return whether an enemy occupied this space
   bool IsUsuable();   // Return whether space is usable
};

// Then you could have an array of these.
// Say it's a four by four grid (I know it'll be much bigger, but just for e.g.)
const int map_row = 4;
const int map_col = 4;
Tile Map[map_row * map_col];  // Array of 16

// This loop will initialise the coords of all 16 tiles
// Would be better in a constructror
int idx = 0;

for(int i=1; i<=map_row;i++)
{
   for(int j=1;j<map_col;j++)
   {
      Map[idx].Init(i,j);
      idx++;
   }
}

Of course, a constructor that sets up the coords would be a better option, but arrays always call the default constructor, so there's not a lot that can be done there. That said, I'm sure there was some sort of uniform initialisation implemented in C++11 that might be worth a look. Although, I have a feeling that if it is available you'll wind up with a long and messy instantiation of your map array.

[Exispistis]

I may implement this later. Thanks.
I have encountered a big problem: I would like to be more specific, but the easiest way to say it is that some of my code doesn't work (as I have never worked on a project using more than one class before (and that class only contained two functions and one variable)), so make mistakes. Could someone correct this code for me, please?

List of errors (all in Enemy.cpp):
"error: expected primary-expression before '.' token"
"error: expected primary-expression before '.' token"
"error: expected primary-expression before '.' token"
"error: lvalue required as left operand of assignment"
"error: expected unqualified-id before '.' token"

LivingObject.cpp:

#include "LivingObject.h"
#include "Enemy.h"
//....
void LivingObject::DetermineFightFirst()
{
    int en=rand()%this->getSpeed(), pl=rand()%Player.getSpeed();
    if(en=pl)
    {
       EnemyFightFirst=rand();
    }
    else
    {
        EnemyFightFirst = en>pl?1:0;
    }
}

void LivingObject::calcDamage()
{
    if(EnemyFightActive=0)
    {
        damage = rand()%Player.getAtk()-rand()%this->getDef();
             }
             else{damage = rand()%this->getAtk()-rand()%Player.getDef();}
}
void LivingObject::takeDamage(){
if(EnemyFightActive=0){this->getHeal()-=damage;}else{Player.getHeal()-=damage;}
}

Enemy.h

#ifndef ENEMY_H
#define ENEMY_H

#define enemyAttackModifier 6
#define enemyDefenceModifier 6
#define enemyMagicModifier 8
#define enemyHealthModifier 7
#define enemyMaximumHealthModifier 7
#define enemyMoneyModifier 8
#define enemySpeedModifier 7

#include "LivingObject.h"
#include "AemRandW.h"
#include "Player.h"

#include <iostream>
#include <cstdlib>
#include <ctime>

using namespace std;


class Enemy: public LivingObject
{
public:
    Enemy()
    {
        srand(time(0));
    };
    void setStats(int a, int b, int c, int d, int e, int f, int g);
    void setApproachText();
    void DetermineFightFirst();
    virtual void attack() = 0;
    ~Enemy();
};

Player.h:

#ifndef PLAYER_H_INCLUDED
#define PLAYER_H_INCLUDED

#include "LivingObject.h"

class Player: public GameObject{
    public:
void setAtk(int a){
atk = a;
}
void setDef(int a){
def = a;
}
void setMag(int a){
mag = a;
}
void setHeal(int a){
heal = a;
}
void setMaxheal(int a){
maxheal = a;
}
void setMon(int a){
mon = a;
}
void setExp(int a){
exp = a;
}
void setSpeed(int a){
speed = a;
}
    protected:
int atk, def, mag, heal, maxheal, mon, exp, speed;
};

LivingObject.h:

#ifndef LIVINGOBJECT_H
#define LIVINGOBJECT_H

#include "GameObject.h"
#include "Player.h"
#include <iostream>
#include <cmath>
#include <cstdlib>
#include <ctime>

class LivingObject: public GameObject
{
    public:
        LivingObject();
        void DetermineFightFirst();
        int getAtk();
        int getDef();
        int getMag();
        int getHeal();
        int getMaxHeal();
        int getMon();
        int getSpeed();
        void calcDamage();
        void takeDamage();
        ~LivingObject();
    protected:
    int atk, def, mag, heal, maxheal, mon, speed, damage;
    bool EnemyFightFirst, EnemyFightActive;
};

There is more code than this in the files, but the irrelevant code has been removed.

I am aware that most people by now would suggest that I try something easier, but I feel that I will learn more effectively from learning how to avoid mistakes than learning every single technique (as this way I am actually putting skills into practice). Good night, everyone (and thanks for all of the help so far)!

[cire]

Given you're supplying the get and set operators for every member, perhaps something along the lines of the following would simplify things:

class LivingObject : public GameObject
{
public:
     struct Stats
     {
           int atk,
                def,
                mag,
                heal,
                maxheal,
                mon,
                speed,
                dmg ;
     };

     const Stats& getStats() const { return stats ; }
     Stats & getStats() { return stats; }

protected:
     Stats stats ;
     bool EnemyFightFirst, EnemyFightActive ;
};

You also don't want to redefine the variables in the derived class. They already exist. The base class destructor is virtual, right?

[Exispistis]

Okay, thanks.
Nobody has ever explained to me how to use the this keyword in functions (other than something like this):

void ClassName::Function
cout << "n = " << n << endl;
cout << "this->n =" << this->n << endl;
cout << "(*this).n = " << (*this).n << endl;

The above example, however, did not teach me how to deal with functions like:

const Stats& getStats() const { return stats ; }

or

Stats & getStats() { return stats; }

Could anyone please explain to me how to use such functions, please?

[Gaminic]

The "this" keyword is an implicit pointer that every Object carries, to point to itself. It's generally unnecessary to use it, but it can lead to clearer code, or solve ambiguity between similarly named variables. Think about this:

struct myStruct {
    int myInt;
    void someFunction(int);
}
void myStruct::someFunction(int myInt) {
 // Assigns the value of parameter-variable myInt member-variable myInt 
    this->myInt = myInt;
}

The first question that comes to mind is "Why not simply rename the parameter?". Like I said: the this pointer is generally unnecessary, hence why it's called "implicit" (if no other variable "myInt" existed, the compiler would assume you meant this->myInt by default). The few cases where it is necessary will reveal themselves if you ever need them.

Secondly, the const keyword in function context:
As you see, it appears twice in the sample code. The first const (at the front of the function) means that the return variable is a const reference to a Stats object. This means nothing can be done with it, except copying. (Officially, it's an "rvalue", because it can only appear on the right side of an operator.)
This is in effect the same as doing Stats getStats() { return stats; }, except that the latter (a "pass-by-value") is more expensive because it requires an extra copy operation [I think].

The second const means that the function won't change anything in the state of the Object (so no member variable is changed during the call of this function). I have no idea if there's a "real" use, but it signifies a conceptual "READ ONLY".

The second version of the function (without the const's) is a regular pass-by-reference. This doesn't return a copy, but the actual object. This means it can be the left-hand side of an operator (i.e. you can assign a value to it, or make changes to it, or use it to call functions, etc). The lack of const's means:
a) The returned object can be changed (thus the first const is dropped).
b) The object's state can change during the call of this function (as the Object can be used in function calls etc).

I hope this is a bit clear; it's always a bit "clumsy" to explain these things.

[Need4Sleep]

think of it like the it has relationship. A gameobject is NOT a player or an enemy, so you really shouldnt make them public. Instead a gameobject HAS a player or enemy so the gameobject should INCLUDE them like so:

class GameObject
{
public:
 //functions
private:
 //vars
 Player PlayerOne(parameters);
 Player PlayerTwo(parameters);
 Enemy Ogre();
 Enemy Wolf();
}

[Disch]

I'm going to cut in here. I still see some design flaws that I feel I should point out, and some advise I don't necessarily agree with.

I'm not saying you need to change anything -- if it's working it's working and that's fine. I just feel I should point this out for your future reference. OOP is a very difficult concept to grasp, but you seem to be making a very strong effort here, so I just want to help guide you on the right path.

If you find yourself writing getters and setters for all your member variables you're doing something wrong. The whole point of a class is that it manages its own state. When you start making getters and setters, you are opening the doors to let other classes manipulate its state which defeats the entire point of having a class in the first place.

Putting all the stats in a struct and making a big getter/setter for that struct isn't much better. Sure it reduces the number of getters and setters, but it still horribly violates the encapsulation you're trying to create.

Furthermore, you are treating the player as a special case. You shouldn't be. At least not in the GameObject class.

void LivingObject::calcDamage()
{
    if(EnemyFightActive=0)
    {
        damage = rand()%Player.getAtk()-rand()%this->getDef();

There are a few things wrong with this.

1) It assumes the player is attacking this object, and that this object is not the player.
2) It assumes there is only 1 player.
3) A base class (LivingObject) has knowledge about its children (Player and Enemy). It shouldn't. The base class should be generic and work the same way for ALL its child types. It shouldn't treat any of them specially. If you need special treatment for a specific child class, you put that code inside that child class.


In this situation, you are having to write separate code for player->enemy attacks and enemy->player attacks, even though they both fundamentally are the same. The goal here, and the whole point of having this hierarchy structure is so you can work in more generic terms. If enemies and players are both game objects, you only need to write code for an object->object attack.


I would do something like this:

class GameObject
{
public:
   int calcDamage(const GameObject& attacker)
   {
      return (rand() % attacker.attack) - (rand() % defense);
   }

   void takePhysicalDamage(const GameObject& attacker)
   {
      health -= calcDamage(attacker);
   }

protected:
   int health;
   int attack;
   int defense;
   // ...
};

/*
  Then, elsewhere in your program, in a broader scope (ie, your 'Game' or 'World' class,
    or your 'main' function .... basically whatever part of your program owns all the enemies
    and the player...)
*/

// if the player is attacking an enemy
enemy.takePhysicalDamage( player );

// or if an enemy is attacking the player
player.takePhysicalDamage( enemy );

Since inheritance forms that "is a" relationship, you can treat Player and Enemy objects as if they were GameObjects, because they are GameObjects. This lets you write generic functions like the above which work with any GameObject, regardless of whether it's a player or a specific kind of enemy.


Also note that GameObject doesn't need to know or care whether it's a player attacking an enemy, or vice versa. It could be a player attacking a player for all it cares.

Also note that since all the stats are being modified in GameObject, you don't need a crapload of getters and setters, because GameObject is just modifying itself like it should.

[Exispistis]

Thanks, I think that I get it now! It makes so much more sense when someone explains it using something that I might do as an example that me trying to understand their example. Also, I am glad that you mention that OOP is difficult to learn, because I thought that it was just my lack of skills (plus the fact that as I can't afford books (and would likely be intimidated by their size) or classes (in addition to the fact that I am too young), I have to find the information myself and it can be a bit scattered at times when using the Internet).

One more thing, then I think that I shall be fine: I have an int variable in my "MapFunctions" class named level (protected) and want to get it (not directly modify it, however) with the "GameFunctions" class. Possibly using a function inside the "MapFunctions" class that returns the value of "level" and accessing that function through a function in the "GameObject" class. How would I do this? A friend function? Something else? The code is below:

GameObject.h:

#include "AemRandW.h"

using namespace std;

class GameObject
{
public:
    GameObject();
    ~GameObject();
protected:
private:
};

AemRandW.h:

using namespace std;

class MapFunctions
{
public:
    MapFunctions();
    void MapRead();
    void SetLevel(int levelvalue);
    int getLevel();
    void FindStart();
    void MapDelete();
    void PlayMap();
    void EquateTextPrint();
    void DirectionCheck();
    ~MapFunctions();
protected:
    int level;
private:
    int ** Map;
    int locx, locy, rows, cols, StartTextSize, DirectionSpecifierCount;
    bool mapCompletion, DirectionSpecifier[4], IgnoreSpecifier;
    string StartText;
};

Note: this file is not very well organized as it is very near to the state that it was before the game structure was changed (as was suggested to me to do so a couple of days ago).

Thanks!
Edit: I'm pretty sure that the answer is no, but is it possible to have an object for a class in another class?

[Disch]

Thanks, I think that I get it now! It makes so much more sense when someone explains it using something that I might do as an example that me trying to understand their example

Yeah. I'm a learn by example guy, myself ;)

Also, I am glad that you mention that OOP is difficult to learn,

It really is. It took me very long to "get it", but once it clicked it just made so much sense. That'll happen to you one of these days. You'll be working on some code and then all of the sudden a lightbulb will go off and you'll finally understand all the things I'm talking about ;P

It just takes time and effort.

One more thing, then I think that I shall be fine: I have an int variable in my "MapFunctions" class named level (protected) and want to get it (not directly modify it, however) with the "GameFunctions" class

Your MapFunctions class kind of confuses me.

A class is not just a collection of functions. It is supposed to represent a "thing". Your Player and Enemy classes represent a clear 'thing', but what exactly does your MapFunctions class reprensent?

I would think it would represent the map, but then why does it have things like locx, locy, directionspecifier, etc? Those sound like they should be part of the player or one of the player's parent classes.



That said, you're going to need to have some kind of way for the game objects to communicate with the map. I would probably have the objects talk to the map, rather than doing it the other way around.

class GameObject
{
//...
  Map* gameMap;

  void Move()
  {
    int newx = /*whatever*/
    int newy = /*whatever*/
    if( gameMap->CanMove( newx, newy ) )
    {
      xpos = newx;
      ypos = newy;
    }
    else
    {
      // can't move here
    }
  }
};

Edit: I'm pretty sure that the answer is no, but is it possible to have an object for a class in another class?

If I'm understanding this question correctly, yes it is definitely possible. In fact you are already doing it.

Remember, std::string is just another class. So:

class Person
{
  std::string name;  // an object inside a class
   // this is "composition" and implies a "has a" relationship.
   //  here, a Person "has a" name.
};

[Exispistis]

I know that different people learn at different speeds, but just for a general idea, how long did it take before you suddenly got it?

The MapFunctions class has not been cleaned up from previously, as I'd decided that it was easier to handle these errors first. I have just seen what you mean by the lack of MapFunctions's representation, so I shall be moving most of the class to either GameObject or Player, as well as making all private variabes protected (and delete mapCompletion altogether as it is not really necessary).

If I'm understanding this question correctly, yes it is definitely possible. In fact you are already doing it.

Remember, std::string is just another class.

I had never considered that, but I can see that it is a class now that you point it out. I was just not sure because I had tried to include an object of MapFunctions in the GameObject class and the compiler stated:

"error: 'MapFunctions' does not name a type"

I have posted the GameObject class below (with the object declaration) so that what I have done wrong can be seen and you can correct me:

GameObject.h:

#ifndef GAMEOBJECT_H
#define GAMEOBJECT_H

#include "AemRandW.h"

using namespace std;

class GameObject
{
public:
    GameObject();
    ~GameObject();
protected:
MapFunctions Mapacc;
};

[firedraco]

The issue is probably that you haven't defined what MapFunctions is. You need to include the header file with the definitions in order to use it.