0
\$\begingroup\$

I'm trying to create a scene manager system for a small framework I'm writing on top of SDL2. I have implemented a scene management system that works, but I am not sure if it is structured how it should be.

What I am doing:

I am following an OO approach where each scene inherits from "IScene", an abstract class with the standard on_load, on_update, etc. methods. These scenes are managed by a scene manager class, which stores a map of int ids and IScene* scene pointers. Here is the code to go with this explanation:

namespace se {
    class IScene;
}

namespace se::managers {
    class SceneManager {
        public:
            SceneManager();
            ~SceneManager();

            void add_scene(int scene_id, IScene* scene);
            void remove_scene(int scene_id);
            IScene* pop(int scene_id);
            IScene* get_scene();
            void set_scene(int scene_id);
        private:
            std::map<int, IScene*> scenes;

            int active_scene;
            IScene* active_scene_ptr;
    };
}

scene.h

namespace se {
    class IScene {
        public:
            IScene(
                IApp* app
            );
            virtual ~IScene();
            virtual void on_load() = 0;
            virtual void on_unload() = 0;
            virtual void on_scene_enter() = 0;
            virtual void on_scene_exit() = 0;
            virtual void on_update() = 0;
            virtual void on_draw() = 0;
        protected:
            IApp* app;
    };
}

The Problem: In order to get this relationship to work, I had to forward declare IScene in scenemanager.h to avoid a cyclic dependency. This is because I want IScene to be able to change the active scene from within its own on_update method. I read some threads that said forward declaration to avoid this issue can be considered a "code smell" because it's a sign of tight coupling.

What I would like to know

  1. Is my approach okay?
  2. What would be a better approach?

Thank you for your time!

P.S. This is my first time ever posting on Stack Exchange. If there's anything about this post that is structured wrong/unclear please let me know so I can fix it in the future.

\$\endgroup\$
1
  • \$\begingroup\$ class SceneManager violates the rule of three. If you declare a destructor, you should also add copy/move constructors and assignments, or get rid of all of them including the destructor. \$\endgroup\$ Commented Nov 24, 2023 at 6:01

3 Answers 3

0
\$\begingroup\$

Forward declaring is totally OK. The alternative would be exactly what you pointed out, cyclic dependencies (assuming that you have #include in your IScene.h). You're saving compile time with your approach. However when you write interfaces you should try to keep them abstract from the rest of the code and not include much in them, hence the name "abstract classes".

\$\endgroup\$
0
\$\begingroup\$

One possible alternative, given that the only reason for the forward declaration is because you want IScene to be able to change the active scene from within its own on_update method, is to have on_update return some data indicating whether or not the scene should be changed, and which scene to change it to, if so.

This option would require implementing the ability to create an IScene given only that data. If you have a fixed set of scenes, then the scene to change to could be represented by an enum. If you want to support an open set of scenes, such that new ones can be added without updating an enum definition, then you would need a more complicated lookup mechanism, if you wanted to pursue this option.

Consider whether this is worse or better than the forward declaration.

\$\endgroup\$
0
\$\begingroup\$

I am far away from being a pro, so excuse me if this is nonsense... But how about seeing a scene as a node and the manager as a kind of graph? The graph/manager could have a method to "search" a scene/node in the map, wich could be called from any scene/node.

I dont see forward declaration as a problem in any kind.

\$\endgroup\$

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .