Array choice based on template specialization

Question

I've got these two template functions responsible of adding/removing actors in-game in GameEngine.h:

template<typename T>
void add(T actor);
template<typename T>
void remove(T actor);

And these arrays:

std::set<std::shared_ptr<Tickable>> tickables;
std::set<std::shared_ptr<Hitbox>> hitboxes;
std::set<std::shared_ptr<sf::Drawable>> drawables;
std::set<std::shared_ptr<EventCatch>> eventCatchers;

Currently, the way i fill these is really... horrible, here's GameEngine.cpp:

template<>
void GameEngine::add<std::shared_ptr<sf::Drawable>>(std::shared_ptr<sf::Drawable> actor)
{
    drawables.insert(actor);
}

template<>
void GameEngine::add<std::shared_ptr<Hitbox>>(std::shared_ptr<Hitbox> actor)
{
    hitboxes.insert(actor);
}

template<>
void GameEngine::add<std::shared_ptr<Tickable>>(std::shared_ptr<Tickable> actor)
{
    tickables.insert(actor);
}

template<>
void GameEngine::add<std::shared_ptr<EventCatch>>(std::shared_ptr<EventCatch> actor)
{
    eventCatchers.insert(actor);
}

template<>
void GameEngine::remove<std::shared_ptr<sf::Drawable>>(std::shared_ptr<sf::Drawable> actor)
{
    drawables.erase(actor);
}

template<>
void GameEngine::remove<std::shared_ptr<Hitbox>>(std::shared_ptr<Hitbox> actor)
{
    hitboxes.erase(actor);
}

template<>
void GameEngine::remove<std::shared_ptr<Tickable>>(std::shared_ptr<Tickable> actor)
{
    tickables.erase(actor);
}

template<>
void GameEngine::remove<std::shared_ptr<EventCatch>>(std::shared_ptr<EventCatch> actor)
{
    eventCatchers.erase(actor);
}

My knowledge about template / smart pointer is limited(yet), is there a better way to do this?
Even if that modifies the arrays, as long as i can call everything is the Loop:

The game loop looks like that in GameEngine.cpp:

void GameEngine::GameLoop()
{
    sf::Clock timer;
    sf::Time tickRate;
    bool run = true;
    Level level(this);

    while (run) {

        clear();

        for (std::shared_ptr<sf::Drawable> drawable : drawables)
            draw(*drawable);

        display();

        sf::Event event;
        while (pollEvent(event))
        {
            switch (event.type) {
            case sf::Event::Closed:
                close();
                run = false;
                break;
            default:
                for (std::shared_ptr<EventCatch> evt : eventCatchers)
                    evt->onEvent(event);
                break;
            }
        }

        for (std::shared_ptr<Tickable> tick : tickables)
            tick->onTick(tickRate.asMicroseconds());

        tickRate = timer.restart();
    }
}

Answer 1

Small rant about question / description

And these arrays: [..]

Be as precise as necessary when describing things. Generalize ("containers") when it helps to make things more clear. But don't say wrong things. These are no arrays, these are sets.

Accessing the sets based on their content type.

std::set<std::shared_ptr<Tickable>> tickables;

The fact that this set contains "tickables" is encoded twice in the line above: The type of that std::set contains the type Tickable and the name (informally) tells us the same. While accessing this set as tickablesis a good and very readable option, this does not lend itself to template programming that much, because the compiler doesn't understand "tickables". It's just another identifier, the meaning (that we humans are able to understand) is lost.

Thus I suggest accessing your sets like set_of<Tickable>(). This is still readable to us, but also provides information for the compiler about the content of the set.

To be able to pull this off I wrap the sets in a helper struct:

template <typename Content>
struct SetWith {
  std::set<Content> set;
};

This only holds a single set (with a single content type). I generalize to multiple content types (and multiple sets) via variable template arguments:

template <typename... Contents>
struct SetsContainer {};

template <typename Content, typename... OtherContents>
struct SetsContainer<Content, OtherContents...>
  : public SetsContainer<OtherContents...>,
    public SetWith<Content>
{};

The first is the template definition that will be used when the template specialization (the second definition above) doesn't match, i.e. when there's no (more) content: SetsContainer<>.

The specialization inherits from the previously defined wrapper (and thus gets a member of type std::set<Content>) and also inherits from a variation of its own type with the Content "removed".

Accessing the sets is now almost trivial:

template <typename C, typename Container>
std::set<C> & set_of(Container & sets) {
    return static_cast<SetWith<C> *>(&sets)->set;
}

// Use:
SetsContainer<int, bool, char> sets;
set_of<bool>(sets).insert(true);

Of course you can also make set_of an (template) member function of a class containing a SetsContainer named sets and drop the parameter.

Your add is then:

template <typename T>
void add(T actor) {
  set_of<typename std::decay<T>::type>().insert(std::forward<T>(actor));
}

Here's an ideone link to play with the above.

Don't throw away useful information

What if you add the very same Tickable twice? Do you want to be informed about this? Because if its added twice when you didn't intent to, this could help you find bugs.

std::set::insert returns useful information, so why throw it away? Better make your add return e.g. the bool returned by insert.

Heavy copies ahead!

The "new" for syntax has its issues, IMO. Do you know that every time you run that (or any of the other) loop ...

for (std::shared_ptr<Tickable> tick : tickables) // ...

... you're creating a copy of every smart pointer in the set. This is not as bad as making a copy of the Tickables, but it still degrades performance: Every time a smart pointer is created it needs to increment the reference counter, and on every destruction it decrements it again.

Better add a reference to avoid those copies:

for (std::shared_ptr<Tickable> & tick : tickables) // ...

This isn't facebook, don't share that much ;)

This is just a guess, but I'd assume that you misuse std::shared_pointer. Think carefully about who owns (for example) a Tickable. Who controls its lifetime?

If it's the GameEngine, then you are far better of using std::set<Tickable>.

The sets are useless

That said, putting std::shared_pointers inside a std::set (with the default compare) means that its operator< is used by the set to prevent duplicates. But, from above link (emphasis mine):

Note that the comparison operators for shared_ptr simply compare pointer values; the actual objects pointed to are not compared.

Thus you can happily write (assuming a copy constructor) ...

auto foo = std::make_shared<Tickable>();
auto bar = std::make_shared<Tickable>(*foo); // Calls copy constructor
add(foo);
add(bar);

... and have two copies of a Tickable in your set.