Component/Strategy Pattern in C++, Design and Implementation

Question

I have a C++ project, using OpenFrameworks for rendering, that bounces a couple of balls (circles) around in a window. They are implemented using the Component Pattern (sometimes referred to as Strategy Pattern) to encapsulate look and behavior respectively. This makes the look (here: Amiga Ball) and the behavior (here: bouncing with gravity) exchangeable at runtime.

The pattern itself is inspired by the Component Pattern chapter in Robert Nystrom's book Game Programming Patterns. Upon pressing a key the behavior of "bouncing with gravity" is exchanged with "bouncing with no-gravity" (the creativity of this is probably the reason I'm programming and not designing). I want to implement the pattern using smart pointers (in this case unique_ptr would do because there is no sharing and one component object is always uniquely held by a GameObject). There are several "ugly" parts in my code related to the use of unique_ptr and the fact that I use polymorphism for the components but seemingly sometimes have to treat them as derived-class objects instead of base-class.

I would like to know if it a good and feasible design and implementation w.r.t. design and implementation of the pattern in general, and the use of smart pointers in particular.

Note: I didn't care for const-correctness here although I should have. So I'm sure there is lots of const to be put in there, but please don't bash me simply for that, except it plays into the other problems.

So here goes...

We have a class GameObject holding the minimal information any object on the screen should have: GameObject.hpp:

    class GameObject {
    public:
        float x;
        float y;

        GameObject(unique_ptr<PhysicsComponent> physics, unique_ptr<GraphicsComponent> graphics);
        GameObject();
        GameObject(float x, float y);

        void update();
        void draw();
        void setPhysicsComponent(unique_ptr<PhysicsComponent> pc);
        void setGraphicsComponent(unique_ptr<GraphicsComponent> pc);
        unique_ptr<PhysicsComponent>& getPhysicsComponent();
        unique_ptr<GraphicsComponent>& getGraphicsComponent();

    private:
        unique_ptr<PhysicsComponent> physics_;
        unique_ptr<GraphicsComponent> graphics_; 
    };

GameObject.cpp:

GameObject::GameObject(unique_ptr<PhysicsComponent> physics, unique_ptr<GraphicsComponent> graphics) : physics_(move(physics)), graphics_(move(graphics)) {}

GameObject::GameObject() {}

GameObject::GameObject(float x, float y) : x(x), y(y) {}

void GameObject::update(){
    physics_->update(*this);
}

void GameObject::draw() {
    graphics_->draw(*this);
}

void GameObject::setPhysicsComponent(unique_ptr<PhysicsComponent> pc) {
    physics_ = std::move(pc);
}

void GameObject::setGraphicsComponent(unique_ptr<GraphicsComponent> gc) {
    graphics_ = std::move(gc);
}

std::unique_ptr<GraphicsComponent>& GameObject::getGraphicsComponent() {
    return graphics_;
}

std::unique_ptr<PhysicsComponent>& GameObject::getPhysicsComponent() {
    return physics_;
}

Then there is the base class for components (in this case lets focus on physics components, as the graphics components are equivalent):

class GameObject;

class PhysicsComponent {
    public:
        virtual ~PhysicsComponent() {std::cout<<"~PhysicsComponent()"<<std::endl;}
        virtual void update(GameObject& obj) = 0;

        void setXSpeed(float xs) {xSpeed = xs;};
        float getXSpeed() {return xSpeed;};
        void setYSpeed(float ys) {ySpeed = ys;};
        float getYSpeed() {return ySpeed;};

    protected:
        float ySpeed;
        float xSpeed;
};

A concrete implementation of the PhysicsComponent looks like this:

class GravityBouncePhysicsComponent : public PhysicsComponent {
    public:
        virtual ~GravityBouncePhysicsComponent()  {std::cout<<"~GravityBouncePhysicsComponent()"<<std::endl;};
        GravityBouncePhysicsComponent();
        GravityBouncePhysicsComponent(float radius, float xSpeed, float ySpeed, float yAccel);
        virtual void update(GameObject& obj);
        void setRadius(float r) {radius = r;};
        float getRadius() {return radius;};
    private:
        // Physics
        float yAcceleration;
        float radius;
};

In the GravityBouncePhysicsComponent.cpp the empty constructor creates such a component with random values for xSpeed and ySpeed (living in the base class) and yAcceleration (making only sense in the derived component class). And then there is another one called FloatBouncePhysicsComponent with similar code.

Now I want to use these classes in the main program, which happens to be the ofApp class in OpenFrameworks. The header file looks like this (some insignificant stuff omitted):

class ofApp : public ofBaseApp{
    public:
        void setup();
        void update();
        void draw();

        void keyPressed(int key);
        // ... lots of other event functions for mouse etc.

    private:
        std::vector<GameObject> balls;
        GameObject createAmigaBall(float x, float y);

        // more variables and some methods here ...

};

Now in the ofApp.cpp file it starts getting ugly. For example in a helper function to create a new ball it looks like this:

GameObject ofApp::createAmigaBall(float x, float y) {
    GameObject go(x,y); // create an empty GameObject on the stack at position x,y
    go.setGraphicsComponent(std::make_unique<AmigaBallGraphicsComponent>());

    if(gravity) {
        go.setPhysicsComponent(std::make_unique<GravityBouncePhysicsComponent>());
        float r = (static_cast<AmigaBallGraphicsComponent*>((go.getGraphicsComponent()).get()))->getRadius();
        (static_cast<GravityBouncePhysicsComponent*>((go.getPhysicsComponent()).get()))->setRadius(r);
    } else {
        go.setPhysicsComponent(std::make_unique<FloatBouncePhysicsComponent>());
        float r = (static_cast<AmigaBallGraphicsComponent*>((go.getGraphicsComponent()).get()))->getRadius();
        (static_cast<FloatBouncePhysicsComponent*>((go.getPhysicsComponent()).get()))->setRadius(r);
    }

    return go; // calls the copy constructor for return value, so we have no dangling pointers or refs
}

Here the AmigaBallGraphicsComponent empty constructor generates a random radius. This radius belongs into this derived class because it is certainly not in every GraphicsComponent. However, this leads to having to clumsily extract the radius from the generated component with ugly casts and accessing the raw pointer, only to do that again to set the same radius in the GravityBouncePhysicsComponent.

And if you think THAT was ugly, check out this snippet:

void ofApp::keyPressed(int key){
    if(key == 'a') {
        if(gravity) {
            for(int i=0; i<balls.size(); i++) {
                unique_ptr<GravityBouncePhysicsComponent> opc (static_cast<GravityBouncePhysicsComponent*>((balls[i].getPhysicsComponent()).release()));
                float r = opc->getRadius();
                float xs = opc->getXSpeed();
                float ys = opc->getYSpeed();
                balls[i].setPhysicsComponent(std::make_unique<FloatBouncePhysicsComponent>(r,xs,ys));
            }
        } else {
                // similar code for the non-gravity case omitted ...
            }
        }
        gravity =!gravity;
    }
}

And, as if that casting-to-derived-classes stuff was not enough, it turned out that this piece of code does not work

auto gc = std::make_unique<AmigaBallGraphicsComponent>();
go.setGraphicsComponent(gc);

while this one does. What the ... ?

go.setGraphicsComponent(std::make_unique<AmigaBallGraphicsComponent>());

Shouldn't this be exactly the same?

Thanks in advance for any insights on this whole mess (or is it?).

Answer 1

I think most of your issues come from the fact that you're letting the ownership (std::unique_ptr) and type erasure (polymorphism) concerns bleed out of your API, which requires a lot of irrelevant lifetime management and downcasting on the user side. getGraphicsComponent (and its buddies) could be implemented like this:

template <class DerivedGraphicsComponent = GraphicsComponent>
auto &getGraphicsComponent() {
    return static_cast<DerivedGraphicsComponent &>(*graphics_);
}

This enables you to write:

float r = go.getGraphicsComponent<AmigaBallGraphicsComponent>().getRadius();

Or to omit the <AmigaBallGraphicsComponent> and receive a plain GraphicsComponent &.

You do lose the ability for GameObject to revoke ownership of one of its components, and return it through a std::unique_ptr, but it doesn't look like you actually need it. The snippet where you use that feature ends up doing:

balls[i].setPhysicsComponent(std::make_unique<FloatBouncePhysicsComponent>(r,xs,ys));

... which destroys the previous component anyway through graphics_'s move-assignment operator. If you do need that functionality, by all means add another function:

template <class DerivedGraphicsComponent = GraphicsComponent>
auto releaseGraphicsComponent() {
    return std::unique_ptr<DerivedGraphicsComponent>(
        static_cast<DerivedGraphicsComponent *>(graphics_.release())
    );
}