Simple sceneManager, problem with c++ polymorphism?


#1

Hello everyone,
I’m try to develop a very simple sceneManager. I know there are many options on the web, I’m simply doing it to better grasp some c++ concepts.
I’m facing a problem with pointers and polymorphism I can’t figure out what it is.
Please bear with me if I’m too verbose with code pasting.
The following image is intended to sum up the situation:


In ofApp.cpp I have the following situation

void ofApp::setup(){
	sm = SceneManager();
	sm.addScene(0, SceneZero()); //<--This doesn't work
	//sm.addScene(0, *new SceneZero());  <--This works but I'd like to avoid dynamic allocation
	sm.setup();
}

void ofApp::update(){
	sm.draw();
}

void ofApp::draw(){
	sm.draw();

}

ofApp.h

class ofApp : public ofBaseApp{

	public:
	    .
	    .
		SceneManager sm;
};

SceneManager.h

#pragma once

#include<unordered_map>
#include "Scene.h"
#include "SceneCounter.h"

class SceneManager{
private:
    //hash table to mantain "storyboard"
	std::unordered_map <int, Scene*> scenes; 
    //currentScene index, had to define it this way to avoid circular dependency
	int * csptr = &SceneCounter::currentScene;

public:
	SceneManager();
	~SceneManager();

	void addScene(int id, Scene & scene);
	void removeScene(int id);
	void setCurrentScene(int id);
	Scene * getCurrentScene();

	void setup();
	void update();
	void draw();
};

SceneManager.cpp

#include "SceneManager.h"

SceneManager::SceneManager(){}

SceneManager::~SceneManager(){}

void SceneManager::addScene(int id, Scene &scene){
	scenes.emplace(id, &scene);
}

Scene * SceneManager::getCurrentScene(){
	return scenes.at(*csptr);
}

void SceneManager::removeScene(int id) {
	scenes.erase(id);
}

void SceneManager::setCurrentScene(int id) {
	*csptr = id;
}

void SceneManager::setup(){
	scenes.at(*csptr)->setup();
}

void SceneManager::update(){
	scenes.at(*csptr)->update();
}

void SceneManager::draw(){
	scenes.at(*csptr)->draw();
}

Scene.h

#pragma once
#include<iostream>
#include "SceneCounter.h"

class Scene{
protected:
	int * csptr = &SceneCounter::currentScene;
public:
	Scene();
	virtual ~Scene();

	virtual void setup();
	virtual void update();
	virtual void draw();
};

Scene.cpp

#include "Scene.h"

Scene::Scene(){}
Scene::~Scene(){}

void Scene::setup(){
	printf("Scene setup");
}

void Scene::update(){
	printf("Scene update");
}

void Scene::draw(){
	printf("Scene draw");
}

SceneZero.h

#pragma once
#include "Scene.h"
class SceneZero :
	public Scene
{
public:
	SceneZero();
	virtual ~SceneZero();

	virtual void setup() override;
	virtual void update() override;
	virtual void draw() override;
};

SceneZero.cpp

#include "SceneZero.h"
#include "ofMain.h"

SceneZero::SceneZero() : Scene(){}
SceneZero::~SceneZero(){}

void SceneZero::setup() {}

void SceneZero::update() {}

void SceneZero::draw() {
	ofBackground(0);
	ofSetColor(242);
	ofDrawCircle(0, 0, 100);
}

#2

Hi, and what errors do you get?
first, remove those empty destructors. That violates the rule of 3.
Then, it kinda doesn’t make sense to store an int pointer for the current scene. Actually just using a vector instead of a map would just have the same effect and easier to use.
store the index of the current scene and that would be all you need to access the current scene pointer.
The SceneCounter class seems to be a bit useless.


#3

Hi @roymacdonald! thanks for taking time to answer me.:slightly_smiling_face:
I completely forgot to attach the error, sorry about that. Here it is

Exception thrown at 0x637A0FE8 in motion2_debug.exe: 0xC0000005: Access violation executing location 0x637A0FE8.

As for the virual destructor I was referring to this article, specifically where the author says “You should always make your destructors virtual if you’re dealing with inheritance”.

Honestly, using the map was just an excuse to learn the API, even though saying it right now sounds quite silly. :sweat_smile:

I do agree on this point aswell, my plan was to have a reference to the SceneManager inside the Scene class, this would generate a circular dependency though. So I had to create an association class to handle this situation. I couldn’t think of any better solution.
Thanks again


#4

Maybe you havent define SceneCounter::currentScene; yet.
And when trying to access csptr it doesn’t exist.

I suggest you to use smart pointers (std::shared_ptr), this scene manager will make a great way to learn those and stay away from raw pointers.


#5

that error becomes useful when in debug mode in your ide as it will point to the line where the exception was thrown. but I wouldn’t be amazed if it came from that int pointer.

as for the destructors, notice that it says that you should make the destructors virtual, meaning that if you happen to need a destructor it should be virtual too and not that you have to always add a destructor.
if you are dealing with raw pointers the circular dependency shouldn’t be a problem, although you will need to take care of destruction of pointers.

as @xumo suggest, try using smart pointers and you can forget about memory management.
Although with smart pointers you need to take care of circular dependency, although it is quite easy.

SceneManager.h

#include "Scene.h"

class SceneManager:  
	public std::enable_shared_from_this<SceneManager> //you need this to correctly deal with 'this'
{
public:
	void addScene(std::shared_ptr<Scene> scene);
protected:
	std::vector<std::shared_ptr<Scene> > scenes;
};

SceneManager.cpp

#include "SceneManager.h"
void SceneManager::addScene(std::shared_ptr<Scene> scene){
	if(scene){//just check that it is valid
		scenes.push_back(scene);
		scene.setup(shared_from_this());//when using smart pointers you should use shared_from_this to get the shared pointer instead of simply this.	
	}
}

Scene.h

class SceneManager;//Forward declare
class Scene{
public:
	void setup(std::shared_ptr<SceneManager> sm);

	std::shared_ptr<SceneManager> getSceneManager(); //always call this method for accesing the scene manager.
protected:
	std::weak_ptr<SceneManager> manager; // this is key to avoid circular reference; using a weak pointer instead of shared. 
};

Scene.cpp

#include "Scene.h"
#include "SceneManager.h" // you need to include here because it was forward declared.

void Scene::setup(std::shared_ptr<SceneManager> sm){
	manager = sm;
}
std::shared_ptr<SceneManager> Scene::getSceneManager(){
	if(!manager.expired()){
        return manager.lock();
    }
    return nullptr;
}

Make sure that you use SceneManager as a shared_ptr and not as a direct instance.

ofApp.h

#include "SceneManager.h"
.....
shared_ptr<SceneManager> SM;

ofApp.cpp

 ofApp::setup(){
    SM = make_shared<SceneManager>();

   SM->addScene(make_shared<Scene>());
   SM->addScene(make_shared<SceneInheritor>());//you can also add classes that inherit from Scene 

    //if you want to upcast a shared pointer use dynamic_pointer_cast.
}

#6

@roymacdonald, @xumo thank you guys for your support. Much appreciated. :slightly_smiling_face: