Why is my "setter" function, stored in a std::map, not setting value correctly?

Question

I'm trying to code a basic game engine, and have been running into a problem regarding dynamic component creation. I have a minimal working example here:

#include <bitset>
#include <variant>
#include <map>
#include <functional>

#include <iostream>

using Property = std::variant<int, unsigned int, bool, double, std::string>;

class Component
{
public:
    std::map<std::string, std::function<void(Property)>> _setters;
    Component() {}

    virtual void DestroyComponent() {}
};

class Transform : public Component {
    public: static std::string name() { return "Transform"; }
    void __set_x(Property p) {
        x = std::get<double>(p);
    }
    public: double x = 0;
    
    public: Transform() {
        _setters["x"] = [=](Property p){this->__set_x(p); };
    }
};

int main()
{
    std::vector<Transform> ts = {Transform()};
    Component* c = &ts[0];

    c->_setters["x"](3.0);
    std::cout << ts[0].x << std::endl;

    return 0;
}

(Note that any weirdness in terms of formatting comes from the fact that most of my components are defined with a lot of macros for convenience.)

This will end up printing 0 and returning, when it should print 3.

Now, the problem is this: I have a list of components of a subclass (here represented as a vector of Transforms) that I need to get one of. At runtime, I don't know what the class is, so I get it by referencing the index of the element as a Component pointer. When my code calls the function linked to by _setters[<property>], it does enter the function as expected (proved by putting print functions inside the lambda and called function), but the value is never set. Through debugging, I've also proved to myself that referencing the index of the vector yields the correct memory address, so the pointer should be correct. So my question is: Why aren't the values being set correctly, and how do I fix it?

Answer 1

The problem is with capturing this then copying your Transform object.

In the Transform() constructor the expression _setters["x"] = [=](Property p){this->__set_x(p); }; captures this. This captures the immediate value this had when this capture occured, it can't capture the notion of "the current object".

When you construct ts you use an initializer list which requires the initializer to be copied. During this copy _setters is copied, including the setter set by the original object's default constructor. The this it captured still points to the original object.

Later, when you call c->_setters["x"](3.0); you successfully run the setter for the Transform pointed to by c, but this setter was obtained by copying another Transform. The setter's captured this still points to the original instance in the initialization list. It doesn't point to c. The setter is trying to set the value of x for an instance that no longer exists leading to Undefined Behaviour.

One possible solution is to implement a copy constructor which overwrites the "x" setter to capture the new instance's this. Example :

Transform(Transform const& copy) : Component(copy) {
    _setters["x"] = [=](Property p){this->__set_x(p); };
}

However, this solution duplicates code and may be brittle in cases where you otherwise overwrite "x" yourself externally. You should review your design, possibly adding a "this" parameter to your setter so any instance can pass its own this to the function.