Game Engine Architecture: what am I doing wrong?

Question

I'm working on a Game Engine (more like a Graphics Engine for Physics simulations) in C++. I know a bit of Unity and I like the concept of GameObject and Component Unity has and I wanted to use that in my project too.

The code structure at the moment is as it follows:

Newtonic::Engine: owns the scene, the engine's message bus and the engine's asset manager.

Newtonic::Scene: has shared_ptrs to actors (Actor is really what in Unity is called GameObject)

Newtonic::Actor: just an object in the scene. It has no property on its own, it's just a container for Behaviours that completely define how the actor behaves and how it is rendered onto the screen

Newtonic::Behaviour: abstract class that specifies how an actor behaves when updating game logic or when rendering the scene

Update and Render calls

When Newtonic::Engine should Update or Render the scene it calls Newtonic::Scene::Update or Newtonic::Scene::Render they in turn call the Update or Render methods on each actor in the scene and finally any actor calls it on any behaviour it has.

The assets manager

The assets manager (Newtonic::Assets) owns all the resources the game needs and stores them in a std::map<std::string, whateverresourcetype>. Resources can be accesed by their id from the assets manager that returns them as a std::weak_ptr (so the ownership isn't shared with the code that requires the resource)

The message bus

This should be used to handle communications between separated parts of the engine. Like communications between the logic and graphics parts of the engine, but it still hasn't got a purpose.

Problems I'm having with this design

• Newtonic::Behaviours need to have access to their parent Newtonic::Actor and to their Newtonic::Behaviour siblings (i.e. the other behaviours on the same actor). One possible way of implementing this would be that every Behaviour store a reference to their parent Actor, but this would create a circular dependecy issue as behaviour.h would need the definitions in actor.h and actor.h already uses behaviour.h obviously. Although I know this can be done, it tastes like bad code design to do this and wanted to know whether I'm missing something myself.

• Newtonic::Engine owns widely used parts of the engine like the assets manager and the message bus (in the future it should be used at least). But no other part of the engine can access Newtonic::Engine, and again passing it to child stuff like Scene creates the circular dependency stuff.

• Batched rendering. At the moment there is a MeshRenderer behaviour that takes weak_ptrs to a Newtonic::Shader and a Newtonic::Mesh. In the Render method this behaviour uses the shader with glUseProgram and binds the mesh's VAO. The problem with this is that if I have like 1000 MeshRenderers that use the same mesh and shader they'll end up binding and unbinding shader and VAO 1000 times, which doesn't seem very performant. Here a solution might be using the message bus to schedule the mesh and shader rendering to a BatchRenderer object (that should be owned by Engine I guess) and render after it all has been scheduled.

Code is at: https://github.com/ekardnam/Newtonic thanks for your time!

Answer 1

Your design of Actor with their Behavior looks very much like some variant of the Actor and Component architecture promoted by Mike McShaffry in this book Game Coding Complete. McShaffry also uses a pointer back from his ActorComponent (similar to your Behavior) to the owning Actor.

Is access from Behavior to Actor a bad design ?

When two classes have a circular dependency, it means that they are tightly coupled. And indeed, good design principles tell us to reduce coupling as much as possible.

But in our specific case, there is obviously a tight interdependency between the abstract concepts of Actor and Behavior, regardless of any specific implementation. In fact, it's hard to even imagine a Behavior independently from an Actor that shall behave.

Therefore, the coupling of the classes in your design is not bad. It only expresses the interdependency that exists the concepts. Moreover this design allows a nice separation of concerns: it's better than having a huge Actor class that would also handle all the behavior and have a lot of unknown/unnoticed/uncontrolled internal dependencies.

What to do with the engine ?

I have not looked into your code, and it's not the purpose of this site to do code review. SO I've limited myself to the description provided in your question.

The fact that the Engine owns large parts but is itself not accessed suggest that it is perhaps too big and has too many responsibilities. For example, why does the Engine own the Scene ?

Maybe it would be worth to decouple some parts. Several ideas come to mind:

• Make scene independent of the engine, and inject the engine into the scene
• use of the mediator pattern to make the different parts around the Engine collaborate
• make the message bus (I suppose it'll handle all the events) the core of the mediation between the parts.

Answer 2

This is not a complete answer but should just show some ideas.

First of all, you don't need to place all includes in your headers file, to improve compiling time and to reduce dependencies you should utilize forward declaration.

You could change your actor.h to this:

#ifndef _ACTOR_H
#define _ACTOR_H

#include <vector>
#include <memory>
#include <functional>


 namespace Newtonic
 {
   class Behaviour;

   class Actor
   {
     std::vector<std::shared_ptr<Behaviour>> m_behaviours;

   public:

     void AddBehaviour(std::shared_ptr<Behaviour> behaviour);

     void Render();

     void Update();
   };
 }

The same is with engine.h you do not need to have all those includes in your header, you can move most of them to the .cpp file:

#include <string>
#include <memory>

#ifndef _WINDOW_H
#define _WINDOW_H

struct GLFWwindow;

namespace Newtonic
{
    class Scene;
    class Assets;
    class MessageBus;

    class Engine
    {
        GLFWwindow *m_window;
        std::unique_ptr<Scene> m_scene;
        std::unique_ptr<Assets> m_assets;
        std::unique_ptr<MessageBus> m_messageBus;

    public:

        void Init();
        void OpenWindow(const char *title);

        Assets & GetAssetsManager();
        MessageBus & GetMessageBus();

        void SetScene(std::unique_ptr<Scene> scene);

        void Loop();
    };

}

You could hide even more implementation details using the PImpl idiom.

The problem with this is that if I have like 1000 MeshRenderers that use the same mesh and shader they'll end up binding and unbinding shader and VAO 1000 times

This sounds like you should be using OpenGL's instanced rendering anyway.