SuperFola / pataro

A generic and modular Roguelike game on top of libtcod - I'll be back soon, just need a break!
25 stars 3 forks source link

Factorizing "Use" components code #7

Closed SuperFola closed 3 years ago

SuperFola commented 3 years ago

The Use components are practically just wrappers to launch an action:

Thus I've started to write a OneTimeUse templated component, so that it can return any type of Action.

The problem arise when the creation of an Action needs more than one argument, currently I've settled on a

template <typename A, typename T>
class OneTimeUse
{
public:
    OneTimeUse(T data): m_data(data)
    {
        m_function = [this](Entity* source, Entity* owner) -> std::unique_ptr<Action> {
            return std::make_unique<A>(source, owner, m_data);
        };
    }

private:
    T m_data;
    std::function<std::unique_ptr<Action>(Entity* source, Entity* owner)> m_function;
};

later on, m_function is called in the .use() method of the component, given the source and owner, and returns the action. I've succeeded in using variadic template and constructing a "capture all + perfect forwarding" lambda, so that we can create functions returning an action with more than a single argument. The problem arise when we need to clone the component.

template <typename A, typename... Args>
class OneTimeUse
{
public:
    OneTimeUse(Args&&... args): m_data(std::make_tuple(std::forward<Args>(args)...))
    {
        m_function = [this](Entity* source, Entity* owner) -> std::unique_ptr<Action> {
            return std::apply([source, owner](auto&&... args) {
                return std::make_unique<A>(source, owner, std::forward<Args>(args)...);
            }, m_data);
        };
    }

protected:
        OneTimeUse<A, Args...>* clone_impl() const override
        {
            // typing problem here, most likely because our m_data didn't keep the type correctly according to Args
            return std::apply([this](auto&&... args) {
                return new OneTimeUse<A, Args...>(std::forward<Args>(args)...);
            }, m_data);
        }

private:
    std::tuple<Args...> m_data;
    std::function<std::unique_ptr<Action>(Entity* source, Entity* owner)> m_function;
};

The question is: should we specialize OneTimeUse to take 0 to 5 templated arguments, or find a work around with variadic templates? In the first sample I copied data because I assumed I was using POD, if we assume we're also receiving POD in the variadic template, no more need for perfect forwarding, thus no more errors. But is it viable to assume that we'll only deal with POD?

SuperFola commented 3 years ago

I forgot to mention it, but some use components need to perform some task related to the engine before being used, for example the fireball is calling engine->pickATile(&x, &y) to interact with the player and receive a position on the map where the fireball should be launched.

Thus, to handle such "pre action" we would need even more code, maybe taking an additional lambda which will be called before creating the action. A new problem now arise : this new lambda needs to set some variables (in the case of the fireball, an x and a y) but we can't know for sure in which order those x and y will be given to the action when constructing it if the action is user defined.

We could maybe add a OneTimeTargetUse templated class which takes care of that, no need for a lambda, only taking a text to display to the user + parameters for the targeting thing, and enforce that the position of the target is given as the first arguments of the action.

(example of what I'm talking about: https://github.com/SuperFola/pataro/blob/master/src/Pataro/Components/Use/Fireball.cpp)