27

I'm currently re-designing my Entity System, for C++, and I have a lot of Managers. In my design, I have these classes, in order to tie my library together. I've heard a lot of bad things when it comes to "manager" classes, perhaps I'm not naming my classes appropriately. However, I have no idea what else to name them.

Most of the managers, in my library, are composed of these classes (although it does vary a little bit):

  • Container - a container for objects in the manager
  • Attributes - attributes for the objects in the manager

In my new design for my library, I have these specific classes, in order to tie my library together.

  • ComponentManager - manages components in the Entity System

    • ComponentContainer
    • ComponentAttributes
    • Scene* - a reference to a Scene (see below)
  • SystemManager - manages systems in the Entity System

    • SystemContainer
    • Scene* - a reference to a Scene (see below)
  • EntityManager - manages entities in the Entity System

    • EntityPool - a pool of entities
    • EntityAttributes - attributes of an entity (this will only be accessible to ComponentContainer and System classes)
    • Scene* - a reference to a Scene (see below)
  • Scene - ties all the managers together

    • ComponentManager
    • SystemManager
    • EntityManager

I was thinking of just putting all the container/pools in the Scene class itself.

i.e.

Instead of this:

Scene scene; // create a Scene

// NOTE:
// I technically could wrap this line in a createEntity() call in the Scene class
Entity entity = scene.getEntityManager().getPool().create();

It would be this:

Scene scene; // create a Scene

Entity entity = scene.getEntityPool().create();

But, I am unsure. If I were to do the latter, that would mean I would have a lot of objects and methods declared inside my Scene class.

NOTES:

  1. An entity system is simply a design that is used for games. It is composed of 3 major parts: components, entities, and systems. The components are simply data, which may be "added" to the entities, in order for the entities to be distinctive. An entity is represented by an integer. Systems contain the logic for an entity, with specific components.
  2. The reason I'm changing my design for my library, is because I think it can be changed quite a lot, I don't like the feel/flow to it, at the moment.
Deduplicator
  • 9,209

4 Answers4

22

Well, I had a read through some of the code you linked to, and your post, and my honest summary is that the majority of it is basically completely worthless. Sorry. I mean, you have all of this code, but you haven't achieved anything. At all. I'm going to have to go into some depth here, so bear with me.

Let's start with ObjectFactory. Firstly, function pointers. These are stateless, the only useful stateless way to create an object is new and you don't need a factory for that. Statefully creating an object is what's useful and function pointers are not useful for this task. And secondly, every object needs to know how to destroy itself, not having to find that exact creating instance to destroy it. And in addition, you must return a smart pointer, not a raw pointer, for the exception and resource safety of your user. Your whole ClassRegistryData class is just std::function<std::unique_ptr<Base, std::function<void(Base*)>>()>, but with the aforementioned holes. It does not need to exist at all.

Then we have AllocatorFunctor- there are already Standard allocator interfaces provided- Not to mention that they're just wrappers on delete obj; and return new T;- which again, is already provided, and they won't interact with any stateful or custom memory allocators which exist.

And ClassRegistry is simply std::map<std::string, std::function<std::unique_ptr<Base, std::function<void(Base*)>>()>> but you wrote a class for it instead of using the existing functionality. It's the same, but completely needlessly global mutable state with a bunch of crappy macros.

What I'm saying here is that you've created some classes where you could replace the entire thing with functionality built from Standard classes, and then made them even worse by making them global mutable state and involving a bunch of macros, which is the worst thing you could possibly do.

Then we have Identifiable. It's a lot of the same story here- std::type_info already performs the job of identifying each type as a run-time value, and it doesn't require excessive boilerplate on the part of the user.

    template<typename T, typename Y>
    bool isSameType(const X& x, const Y& y) { return typeid(x) == typeid(y); }
    template <class Type, class T>
    bool isType(const T& obj)
    {
            return dynamic_cast<const Type*>(std::addressof(obj));
    }

Problem solved, but without any of the preceeding two hundred lines of macros and whatnot. This also marks your IdentifiableArray as nothing but std::vector but you have to use reference-counting even if unique ownership or non-ownership would be better.

Oh, and did I mention that Types contains a bunch of absolutely useless typedefs? You gain literally no advantage whatsoever over using the raw types directly, and lose a good chunk of readability.

Next up is ComponentContainer. You can't choose the ownership, you can't have separate containers for derived classes of various components, you can't use your own lifetime semantics. Delete without remorse.

Now, the ComponentFilter might actually be worth a little, if you refactored it a lot. Something like

class ComponentFilter {
    std::unordered_map<std::type_info, unsigned> types;
public:
    template<typename T> void SetTypeRequirement(unsigned count = 1) {
        types[typeid(T)] = count;
    }
    void clear() { types.clear(); }
    template<typename Iterator> bool matches(Iterator begin, Iterator end) {
        std::for_each(begin, end, [this](const std::iterator_traits<Iterator>::value_type& t) {
            types[typeid(t)]--;
        });
        return types.empty();
    }
};

This doesn't deal with classes derived from the ones you're trying to deal with, but neither did yours.

The rest of this is pretty much the same story. There is nothing here that could not be done much better without the use of your library.

How would you avoid managers in this code? Delete basically all of it.

DeadMG
  • 36,914
19

DeadMG is spot on on the specifics of your code but I feel like it misses a clarification. Also I don't agree with some of his recommendation that don't hold in some specific contexts like most high performance-videogame development for example. But he's globally right that most of your code is not useful right now.

As Dunk says, Manager classes are called like this because they "manage" things. "manage" is like "data" or "do", it's an abstract word that can contain almost anything.

I was mostly in the same place than you around 7 years ago, and I started to think that there was something wrong in my way of thinking because it was so much efforts to code to do nothing yet.

What I changed to fix this is to change the vocabulary I use in code. I totally avoid generic words (unless it's generic code, but it's rare when you're not making generic libraries). I avoid to name any type"Manager" or "object".

The impact is direct in code: it forces you to find the right word corresponding to the real responsibility of your type. If you feel like the type does several things (keep an index of Books, keep them alive, create/destroy books) then you need different types that each will have one responsibility, then you combine them in the code that uses them. Sometime I need a factory, sometime not. Sometime I need a registry, so I setup one (using standard containers and smart pointers). Sometime I need a system that is composed of several different sub system so I separate everything as much as I can so that each part do something useful.

Never name a type "manager" and make sure all your type have a single unique role is my recommendation. It might be hard to find names sometime, but it's one of the hardest thing to do in programming generally

Klaim
  • 14,902
  • 4
  • 51
  • 62
10

The problem with Manager classes is that a manager can do anything. You can't read the class name and know what the class does. EntityManager....I know it does something with Entities but who knows what. SystemManager...yep, it manages the system. That's very helpful.

My guess is that your manager classes probably do a lot of things. I'll bet if you were to segment those things into closely related functional pieces, then you'll probably be able to come up with class names related to the functional pieces that anybody can tell what they do simply by looking at the class name. This will make for a far easier to maintain and understand design.

Dunk
  • 5,099
4

I actually don't think Manager is so bad in this context, shrug. If there's one place where I think Manager is forgivable, it would be for an entity-component system, since it is really "managing" the lifetimes of components, entities, and systems. At the very least that's a more meaningful name here than, say, Factory which doesn't make as much sense in this context since an ECS does genuinely do a bit more than create things.

I suppose you could use Database, like ComponentDatabase. Or you might just use Components, Systems, and Entities (this is what I personally use and mainly just because I like terse identifiers though it admittedly conveys even less information, perhaps, than "Manager").

The one part I find a bit clumsy is this:

Entity entity = scene.getEntityManager().getPool().create();

That's a whole lot of stuff to get before you can create an entity and it feels like the design is leaking implementation details a bit if you have to know about entity pools and "managers" to create them. I think things like "pools" and "managers" (if you stick to this name) should be implementation details of the ECS, not something exposed to the client.

But dunno, I have seen some very unimaginative and generic uses of Manager, Handler, Adapter, and names of this sort, but I think this is a reasonably forgivable use case when the thing called "Manager" is actually responsible for "managing" ("controlling?", "handling?") the lifetimes of objects, being responsible for creating and destroying and providing accessing to them individually. That's the most straightforward and intuitive use of "Manager" for me at least.

That said, one general strategy to avoid "Manager" in your name is just take out the "Manager" part and add an -s at the end. :-D For example, let's say you have a ThreadManager and it's responsible for creating threads and providing information about them and accessing them and so forth, but it doesn't pool threads so we can't call it ThreadPool. Well, in that case, you just call it Threads. That's what I do anyway when I'm unimaginative.

I'm kinda reminded of back when the analogical equivalent of "Windows Explorer" was called "File Manager", like so:

enter image description here

And I actually think "File Manager" in this case is more descriptive than "Windows Explorer" even if there's some better name out there that doesn't involve "Manager". So at the very least please don't get too fancy with your names and start naming things like "ComponentExplorer". "ComponentManager" at least gives me a reasonable idea of what that thing does as someone used to working with ECS engines.