c++ - Am I on the right track with this component architecture?

I've recently decided to revamp my game architecture to get rid of deep class hierarchies and replace them with configurable components. The first hierarchy I'm replacing is the Item hierarchy and I would like some advice to know if I'm on the right track.

Previously, I had a hierarchy that went something like this:

Item -> Equipment -> Weapon
                  -> Armor

                  -> Accessory
     -> SyntehsisItem
     -> BattleUseItem -> HealingItem
                      -> ThrowingItem -> ThrowsAsAttackItem

Needless to say it was starting to get messy and these was no easy solution to items that needed to be multiple types (i.e. some equipment is used in item synthesis, some equipment is throwable, etc.)

I then attempted to refactor and place functionality into the base item class. But then I was noting that the Item had alot of unused/superfluous data. Now I'm trying to do a component like architecture, at least for my items before attempting to do so to my other game classes.

Here's what I'm currently thinking for the component setup:

I have a base item class that has slots for various components (i.e. an equipment component slot, a healing component slot, etc. as well as a map for arbitrary components) so something like this:

class Item

{
    //Basic item properties (name, ID, etc.) excluded
    EquipmentComponent* equipmentComponent;
    HealingComponent* healingComponent;
    SynthesisComponent* synthesisComponent;
    ThrowComponent* throwComponent;
    boost::unordered_map > AdditionalComponents;
} 

All item components would inherit from a base ItemComponent class, and each Component type is responsible for telling the engine how to implement that functionality. i.e. the HealingComponent tells the battle mechanics how to consume the item as a healing item, while the ThrowComponent tells the battle engine how to treat the item as a throwable item.

The map is used to store arbitrary components that are not core item components. I'm pairing it with a bool to indicate whether the Item Container should manage the ItemComponent or if it's being managed by an external source.

My idea here was that I define the core components used by my game engine up front, and my item factory would assign the components that the item actually has, otherwise they are null. The map would contain arbitrary components that would generally be added/consumed by scripting files.

My question is, is this a good design? If not, how can it be improved? I considered grouping all components into the map, but using string indexing seemed unecessary for the core item components

Answer

It seems like a very reasonable first step.

You're opting for a combination of generality (the "additional components" map) and lookup performance (the hard-coded members), which may be a bit of a pre-optimization -- your point concerning the general inefficiency of string-based look-up is well-made, but you can alleviate that by choosing to index components by something faster to hash. One approach might be to give each component type a unique type ID (essentially you're implementing lightweight custom RTTI) and index based on that.

Regardless, I would caution you to expose a public API for the Item object that allows you ask for any component -- the hardcoded ones and the additional ones -- in a uniform fashion. This would make it easier to change the underlying representation or balance of hardcoded/non-hardcoded components without having to refactor all the item component clients.

You might also consider providing "dummy" no-op versions of each of the hard-coded components and ensure that they are always assigned -- you can then use reference members instead of pointers, and you will never need to check for a NULL pointer before interacting with one of the hard-coded components classes. You will still incur the cost of the dynamic dispatch to interact with that component's members, but that would occur even with pointer members. This is more of a code cleanliness issue because the performance impact will be negligible in all likelihood.

I don't think it's a great idea to have two different kinds of lifetime scopes (in other words, I don't think the bool you have in the additional component map is a great idea). It complicates the system and implies that destruction and resource release isn't going to be terribly deterministic. The API to your components would be much clearer if you opted for one lifetime management strategy or the other -- either the entity manages the component lifetime, or the subsystem that realizes components does (I prefer the latter because it pairs better with the outboard component approach, which I will discuss next).

The big downside I see with your approach is that you are clumping all of the components together in the "entity" object, which actually isn't always the best design. From my related answer to another component-based question:

Your approach of a using a big map of components and an update() call in the game object is quite sub-optimal (and a common pitfall for those first building these sorts of systems). It makes for very poor cache coherency during the update and doesn't allow you to take advantage of concurrency and the trend towards SIMD-style process of large batches of data or behavior at once. It's often better to use a design where the game object doesn't update its components, but rather the subsystem responsible for the component itself updates them all at once.

You're essentially taking the same approach by storing the components in the item entity (which is, again, an entirely acceptable first step). What you may discover is that the bulk of the access to components you are concerned about the performance of is just to update those, and if you elect to use a more outboard approach to component organization, where the components are kept in a cache-coherent, efficient (for their domain) data structure by a subsystem that understands their needs the most, you can achieve much better, more parallelizable update performance.

But I point this out only as something to consider as a future direction -- you certainly don't want to go overboard overengineering this; you can make a gradual transition through constant refactoring or you may discover your current implementation meets your needs perfectly and there's no need to iterate on it.