So, I've just figure out that my Actor Component Model is flawed by reading L. Spiro posts saying that the game logic handles all the logic of a Character/Player/Node (that actually make a lot of sense).

In my simple implementation I have a IBehaviour class that goes in the Actor class, but AI and Game Logic is supposed to be in a AI class (the Game class in that case). Do you think the following approach is +- correct?

class Character : public Behaviour
{
public:
enum StateMask
{
CHARACTER_CAN_JUMP,
CHARACTER_CAN_FLY
};

Character() : mask(0) { }

virtual void Update(Actor* _pActor)
{
if ( mask & CHARACTER_CAN_JUMP ) //just an example
{
//Do something with jump code
}
}
private:
bool mask;
};

class Game
{
public:

void Update()
{

for ( size_t i = 0; i < characters.size(); ++i )
{

if ( SOME_CONDITION )
{
characters[i]->SetMask( characters[i]->GetMask() | CHARACTER_CAN_JUMP ); //just an example
}

}

}
private:

std::vector characters;

};

class Actor
{
public:
void Update( const Time& _time )
{
pBehaviour->Update( this );
}
private:

IBehaviour* pBehaviour;
};

In this case, I'm removing all the Behaviour stuff and putting into the AI class (the Game class in this case).
Then, I'll pass all data related to the Game in a game_config.xml file. When detects a Character, it's added to be managed by it, and adds a new Actor to the Scene.

Whenever my Actor gets created/destroyed, I need to notify to the Game class to remove the pBehaviour, but by doing this, I'm going in the wrong side of the Single Responsability Principle. Now my Scene knows about the Game class (the AI class (the Game class in this case) ).

Any good advice will be instantaneously voted up.

Not a comment on your organization, but on the implementation of your state mask. It appears you're trying to implement a bitmask for character attributes with binary ORs ("|") not logical ORs ("||"). A more common implementation:

enum {CAN_JUMP = 1, CAN_FLY = 2, CAN_OTHER = 4, ...=8 }; // actual bit positions within the mask
...
unsigned int mask;

Remember, if you add enums beyond 2, without specifying the value, they will continue with 3, 4,... Those masks will be binary 0011, 0100, etc. What you appear to be doing would be better implemented by actual bitmasks for each bit position: 0001, 0010, 0100, 1000, etc.

I know this. I posted without the binary format just to keep it simple.

Veering off topic, but...

I know this. I posted without the binary format just to keep it simple.

If you're using recent-ish versions ofGCC or Clang or using Visual Studio 2013 + November CTP, note that a simple trick is to use constexpr function as a shorthand to make spelling out these enums easier:

constexpr unsigned bit(unsigned index) { return 1 << index; }

enum class foo { None, A = bit(0), B = bit(1), C = bit(2), D = bit(0)|bit(3), E };
// None = 0x0000, A = 0b0000, B = 0b0001, C = 0b0010, D = 0b0101, E = 0b0110

I also recommend a macro to turn enum classes (strong enums) into flag-like objects:

#define MAKE_FLAGS(E) \
using E##_t = typename std::underlying_type<E>::type; \
constexpr E operator|(E lhs, E rhs) { return (E)((E_t)lhs | (E_t)rhs); } \
constexpr E operator&(E lhs, E rhs) { return (E)((E_t)lhs & (E_t)rhs); } \
constexpr E operator^(E lhs, E rhs) { return (E)((E_t)lhs ^ (E_t)rhs); } \
constexpr E operator|=(E& lhs, E rhs) { return lhs = lhs | rhs; } \
constexpr E operator&=(E& lhs, E rhs) { return lhs = lhs & rhs; } \
constexpr E operator^=(E& lhs, E rhs) { return lhs = lhs ^ rhs; } \
constexpr E operator~(E v) { return (E)(~(E_t)lhs); } \
constexpr bool isset(E bits, E mask) { return ((E_t)bits & (E_t)mask) != E_t{}; } \
constexpr void set(E& bits, E mask) { return bits |= mask; } \
constexpr void clear(E& bits, E mask) { return bits &= ~mask; }

MAKE_FLAGS(foo); // defines operators for foo and also a foo_t you can ignore

foo flags = foo::A | foo::C;
flags &= foo::A | foo::B;
if (isset(flags, foo::B))
  clear(flags, foo::B)

You can also accomplish the above with a template wrapper class instead of a macro, though then you end up with two different names: the wrapper name and the enum namespace name.

I have a bits.h that defines the macro, the helper functions like bit(), and so on. Lots of convenience and you can take advantage of the added safety of C++11 strong enums while still using them as convenient flag types.

Why don't you guys donate $5 bucks to get a up vote?

Just kidding...

Whenever my Actor gets created/destroyed, I need to notify to the Game class to remove the pBehaviour, but by doing this, I'm going in the wrong side of the Single Responsability Principle. Now my Scene knows about the Game class (the AI class (the Game class in this case) ).

Why is your scene destroying Actors?

Not a comment on your organization, but on the implementation of your state mask. It appears you're trying to implement a bitmask for character attributes with binary ORs ("|") not logical ORs ("||"). A more common implementation:

enum {CAN_JUMP = 1, CAN_FLY = 2, CAN_OTHER = 4, ...=8 }; // actual bit positions within the mask
...
unsigned int mask;

See reply by SeanMiddleditch. Always prefer (1 << x), either hand-coded or via a macro or constant expression over raw numbers for bit-based enumerations. One of the things I would propose to the new C++ standard is to add bitenum { A, B, C }; to do this for us as it is very common.

In my simple implementation I have a IBehaviour class that goes in the Actor class, but AI and Game Logic is supposed to be in a AI class (the Game class in that case). Do you think the following approach is +- correct?

Although I said the game knows about the game logic, I suppose I was technically talking about each game states, not the Game class itself.
CMainMenuState and CCreditsState will have entirely different logic from CMainGameState, and indeed game logic could change between parts of the main game (see Sonic the Hedgehog 2 which mixed primitive 3D gameplay between its side-scrolling gameplay).
The game state is what decides the logic for each part of the game.


Other than that, what you are implementing is a state machine. You can swap behavior classes in and out of an actor and change how it behaves, so a single actor can have one of many “behaviors” at any given time. This is mostly useful for AI, not a player-controlled character (the human’s mind is the state machine that decides what behaviors to inflict upon his or her in-game persona). While it provides a fairly nice way to encapsulate various enemy behaviors, be warned that it can get tedious and possibly become spaghetti trying to code them all one-by-one as you figure out what code needs to moved around to avoid duplication etc.


The biggest problem with your implementation (I won’t get into the circular dependency, which I would solve by making IBehavior know about ActorBase rather than Actor, or make IBehavior know about Actor but only AIActor has an IBehavior, etc.) is that you are setting flags to indicate potentially valid states.
The only time you need to know if the player can jump is when the player tries to jump. In order to figure out “just in case” a player can jump you are doing some kind of ground test etc. every frame when 99% of the time that information isn’t needed on that frame.
Somewhere in the game logic (the state class that handles the main gameplay) there simply needs to be a boolean indicating, “Yes, the player can jump now,” or, “No, the player cannot jump now,” which is tested only when the player tries to jump, and immediately handled appropriately (not saved as a flag to be deferred for later processing).
Most (if not all) “can I perform this action” tests are done similarly, otherwise you will get a ridiculous explosion of state-setting tests which will invariably lead to bugs. While making Dolphin Trainer DS, my coworker tried to do this, and as a result there are out-lying cases where the dolphin would land back in the water after a jump but fail to correct its alignment and start swimming with its nose pointing down.

For this reason, while it was important to note that your enumerations were not actually bit masks, actually you can just get rid of it entirely.
Bit masks are more suitable for keeping track of character statuses. I don’t mean potential statuses such as “can jump” but currently active statuses such as “power jump is active”.


L. Spiro

Understood. Makes more sense. What have fucked everything is that an Interface (Behaviour) knows about a concrete type (Actor) that doesn't have nothing to do with.

class AIActor is a bag of behaviours. Actor can have a AIActor.

AIActor updates an Actor.

class AIActor
{
public:
    void Update(Actor* _pActor)
    {
         if ( !behaviours.empty() )
         {
            behaviours.top()->Update( _pActor );
         }
    }
private:
     std::stack<Behaviour*> behaviours;
};

Whenever my Actor gets created/destroyed, I need to notify to the Game class to remove the pBehaviour, but by doing this, I'm going in the wrong side of the Single Responsability Principle. Now my Scene knows about the Game class (the AI class (the Game class in this case) ).

Why is your scene destroying Actors?

Because manages them.