It isn't about whether or not your interfaces are syntactically identical, its about whether they are semantically identical in some context, and whether the complete interface provides all of the functionality you require. Because you need to know the type from outside the class, I presume you are implementing some additional functionality outside the class' interface proper. This almost always means that you're interface as defined is not sufficient for your needs, or that you're abusing inheritance for code-reuse.
Sometimes a light-weight ID token is a reasonable solution, but it kinda just sounds like you're afraid of the RTTI boogeyman. Perhaps if you explain more about what this ID is used for we can suggest a better solution to your problem.
'virtual static const'
Sometimes a light-weight ID token is a reasonable solution, but it kinda just sounds like you're afraid of the RTTI boogeyman. Perhaps if you explain more about what this ID is used for we can suggest a better solution to your problem.
Thank you.
Here's the current imp. I haven't yet applied Alvaro's ident as vfunc idea.
[source lang="cpp"]#pragma once
//"vcl_Scene.h"
#include "ns_scenemgr.h"
//When adding a new 'Scene' inheritor be sure
//to add its enumeration value to SceneID,
//add a case in the SceneMgr::update() switch
//and include the Scene's header in ns_SceneMgr.cpp.
//These locations are marked and can be found by
//searching for the *~* comment token.
//Also remember to set the 'ident' base member in
//the class' constructor.
class Scene {
public:
virtual ~Scene() {};
virtual int initialize() = 0;
virtual int update() = 0;
SceneID ident;
};
[/source]
[source lang="cpp"]#pragma once
//"ns_SceneMgr.h"
enum SceneID : int {
NO_SCENE = 0,
// *~*Begin scene ident enumerations
SCENE_TEST,
// End scene ident enumerations
};
namespace SceneMgr {
extern SceneID sceneID;
int initialize(SceneID startingScene);
int update();
};
[/source]
[source lang="cpp"]
//"ns_SceneMgr.cpp"
#include "ns_scenemgr.h"
#include "vcl_Scene.h"
#include <memory>
// *~*Begin scene header inclusions
#include "Scene_Test.h"
// End scene header inclusions
using namespace std;
//This public variable can be changed to cause the
//SceneMgr to switch to a different scene.
SceneID SceneMgr::sceneID;
static auto_ptr<Scene> g_sceneObject(NULL);
//This private class is used to create a 'dummy'
//object for bootstrapping the scene manager.
class Scene_Null : public Scene {
public:
Scene_Null() {ident = (SceneID)-1;}
virtual ~Scene_Null() {}
virtual int initialize() {return 0;}
virtual int update() {
MessageBoxA(0, "Attempted to update the null Scene.", 0, 0);
return -1;
}
};
int SceneMgr::initialize(SceneID startingScene) {
sceneID = startingScene;
try{g_sceneObject.reset(new Scene_Null);}
catch(bad_alloc) {return -1;}
return 0;
}
int SceneMgr::update() {
if(sceneID != g_sceneObject->ident) {
try {
switch(sceneID) {
// *~*Begin Scene construction cases
case SCENE_TEST: g_sceneObject.reset(new Scene_Test); break;
// End Scene construction cases
case NO_SCENE: PostQuitMessage(0); return 0;
default: MessageBoxA(0, "Invalid SceneID.", 0, 0); return -1;
};
}
catch(bad_alloc) {
MessageBoxA(0, "Failed allocation for new Scene object.", 0, 0);
return -1;
}
if(g_sceneObject->initialize()) {return -1;}
}
return g_sceneObject->update();
}
[/source]
To apply C++ RTTI here would require that all Scene derivatives include one another's headers in order to signal the type of the new derivative to switch to. The overwhelming problem with C++ RTTI is that while it's certainly aware of the type at runtime it's not aware of the types available at code-writing time. If the RTTI functionality provided a token that could be mapped to an enum value or something then that would be fine. (A predictable value that I could actually work with instead of an object that I have no use for.)
Maybe I'm just misinterpreting the purpose of a Scene object, but wouldn't this be better served by a data-driven design instead of trying to do type magic on a code implementation of all your features?
The Scene derivatives encapsulate 'scenes' in the game such as Scene_Map or Scene_Menu. The initialize() member loads resources for the objects owned by the Scene (Bitmap, Sprite, Sound, etc) and sets initial values. The update() member processes a single logical frame. If I'm on the map Scene (walking around the world map in an RPG for instance) and I hit the 'menu' button to open the menu then something like this (in Scene_Map's update() member) would handle that eventuality:
[source lang="cpp"]if(Input::trigger(Input::CANCEL)) {
SceneMgr::sceneID = SCENE_MENU;
}[/source]
When the update() function exits execution will then exit SceneMgr::update() and return the the main game loop, which will perform rendering based on the game state, poll input, run all the pending messages through the WinProc and then re-enter SceneMgr::update() (unless something broke the main loop, such as PostQuitMessage(0) breaking the loop by the return value from the function that calls the WinProc, or input polling indicating that the window lost focus).
When SceneMgr::update() sees that the value of sceneID no longer matches the value returned by the current Scene object it will destroy the object, freeing all its resources, and then switch on the new value, either creating a new Scene object to run or else quitting the program (via the NO_SCENE enum value).
Here's the code for Scene_Test, which is a simple test that I'm using while I'm getting the engine all in place:
[source lang="cpp"]//"Scene_Test.h"
#include "vcl_Scene.h"
#include "cl_Sound.h"
#include "cl_Bitmap.h"
#include "cl_Sprite.h"
class Scene_Test : public Scene {
public:
Scene_Test();
virtual ~Scene_Test();
virtual int initialize();
virtual int update();
private:
Sound m_snd;
Bitmap m_bmp;
Sprite m_spr;
};
/////////////////////////////////////////////////////
//"Scene_Test.cpp"
#include "Scene_Test.h"
#include "ns_Audio.h"
#include "ns_Input.h"
#include "ns_WinMgr.h"
Scene_Test::Scene_Test() {
ident = SCENE_TEST;
}
Scene_Test::~Scene_Test() {
//NOP
}
int Scene_Test::initialize() {
if(Audio::loadTrackFromFile("song.ogg")) {return -1;}
Audio::playCurrentTrack();
if(m_snd.loadFromFile("tweet.ogg")) {return -1;}
if(m_bmp.loadFromFile("neut.png")) {return -1;}
m_spr.set_bitmap(m_bmp);
int pos[2] = {
(WinMgr::get_screenWidth() / 2) - (m_bmp.get_width() / 2),
(WinMgr::get_screenHeight() / 2) - (m_bmp.get_height() / 2)
};
m_spr.set_xy(pos);
return 0;
}
int Scene_Test::update() {
if(Input::press(Input::LEFT)) {m_spr.set_x(m_spr.get_x() - 2);}
if(Input::press(Input::RIGHT)) {m_spr.set_x(m_spr.get_x() + 2);}
if(Input::press(Input::UP)) {m_spr.set_y(m_spr.get_y() - 2);}
if(Input::press(Input::DOWN)) {m_spr.set_y(m_spr.get_y() + 2);}
if(Input::trigger(Input::ACTION)) {m_snd.play();}
if(Input::trigger(Input::CANCEL)) {SceneMgr::sceneID = NO_SCENE;}
return 0;
}
[/source]
I don't really know what you mean by data-driven design in this case. If it's applicable could you elaborate?
[source lang="cpp"]if(Input::trigger(Input::CANCEL)) {
SceneMgr::sceneID = SCENE_MENU;
}[/source]
When the update() function exits execution will then exit SceneMgr::update() and return the the main game loop, which will perform rendering based on the game state, poll input, run all the pending messages through the WinProc and then re-enter SceneMgr::update() (unless something broke the main loop, such as PostQuitMessage(0) breaking the loop by the return value from the function that calls the WinProc, or input polling indicating that the window lost focus).
When SceneMgr::update() sees that the value of sceneID no longer matches the value returned by the current Scene object it will destroy the object, freeing all its resources, and then switch on the new value, either creating a new Scene object to run or else quitting the program (via the NO_SCENE enum value).
Here's the code for Scene_Test, which is a simple test that I'm using while I'm getting the engine all in place:
[source lang="cpp"]//"Scene_Test.h"
#include "vcl_Scene.h"
#include "cl_Sound.h"
#include "cl_Bitmap.h"
#include "cl_Sprite.h"
class Scene_Test : public Scene {
public:
Scene_Test();
virtual ~Scene_Test();
virtual int initialize();
virtual int update();
private:
Sound m_snd;
Bitmap m_bmp;
Sprite m_spr;
};
/////////////////////////////////////////////////////
//"Scene_Test.cpp"
#include "Scene_Test.h"
#include "ns_Audio.h"
#include "ns_Input.h"
#include "ns_WinMgr.h"
Scene_Test::Scene_Test() {
ident = SCENE_TEST;
}
Scene_Test::~Scene_Test() {
//NOP
}
int Scene_Test::initialize() {
if(Audio::loadTrackFromFile("song.ogg")) {return -1;}
Audio::playCurrentTrack();
if(m_snd.loadFromFile("tweet.ogg")) {return -1;}
if(m_bmp.loadFromFile("neut.png")) {return -1;}
m_spr.set_bitmap(m_bmp);
int pos[2] = {
(WinMgr::get_screenWidth() / 2) - (m_bmp.get_width() / 2),
(WinMgr::get_screenHeight() / 2) - (m_bmp.get_height() / 2)
};
m_spr.set_xy(pos);
return 0;
}
int Scene_Test::update() {
if(Input::press(Input::LEFT)) {m_spr.set_x(m_spr.get_x() - 2);}
if(Input::press(Input::RIGHT)) {m_spr.set_x(m_spr.get_x() + 2);}
if(Input::press(Input::UP)) {m_spr.set_y(m_spr.get_y() - 2);}
if(Input::press(Input::DOWN)) {m_spr.set_y(m_spr.get_y() + 2);}
if(Input::trigger(Input::ACTION)) {m_snd.play();}
if(Input::trigger(Input::CANCEL)) {SceneMgr::sceneID = NO_SCENE;}
return 0;
}
[/source]
I don't really know what you mean by data-driven design in this case. If it's applicable could you elaborate?
Is the purpose of choosing a new state within the update to ensure that the 'old' state doesn't get deleted as soon as g_sceneObject is set to a new state?
If that's the primary cause of this, then you can solve the whole mess by making g_sceneObject a shared_ptr, and creating copy of the shared_ptr to it as the first step of update() -- this way if you overwrite g_sceneObject with the new state, the reference count on the copy of the old state will be decremented, and when you leave update it will decrement again and be deleted. The next time through you'll have the 'new' state.
From there you can move g_sceneObject into the state manager, and initialize it to a known-good state on construction, then you have no need for Scene_Null or NO_SCENE, because you can then assume that if m_sceneObject (or whatever you name it after moving it into the scene manager) is null you've reached the terminal state.
You'll need to provide a method to change SceneMgr's m_sceneObject member, and you can initialize the new state then and there (preferably by constructor rather than / in conjunction with initialize()), instead of in the update function -- in the SceneMgr::update function, m_sceneObject is either null (and your game has reached the terminal state), or valid and fully initialized.
Update becomes something like:
int SceneMgr::update() {
shared_ptr<Scene> scene = m_sceneObject;
return scene->update();
}
You probably won't need the SceneID enumeration at all, and scene manager won't need to know about all the different scenes that exist at all.
You'll have to give scenes a way to tell SceneMgr to change the scene -- you could pass each scene a reference to SceneManager (but that's kinda gross), you could pass a call-back function that changes it (which is better), or you could re-jigger things so that Scene::Update returns the next scene, or itself if its to stay in the same state -- shared_ptr should take care of all the reference count management automatically. A scene that kicks of any new scenes will have to know about those scenes and those scenes only, which is the way it should be.
If that's the primary cause of this, then you can solve the whole mess by making g_sceneObject a shared_ptr, and creating copy of the shared_ptr to it as the first step of update() -- this way if you overwrite g_sceneObject with the new state, the reference count on the copy of the old state will be decremented, and when you leave update it will decrement again and be deleted. The next time through you'll have the 'new' state.
From there you can move g_sceneObject into the state manager, and initialize it to a known-good state on construction, then you have no need for Scene_Null or NO_SCENE, because you can then assume that if m_sceneObject (or whatever you name it after moving it into the scene manager) is null you've reached the terminal state.
You'll need to provide a method to change SceneMgr's m_sceneObject member, and you can initialize the new state then and there (preferably by constructor rather than / in conjunction with initialize()), instead of in the update function -- in the SceneMgr::update function, m_sceneObject is either null (and your game has reached the terminal state), or valid and fully initialized.
Update becomes something like:
int SceneMgr::update() {
shared_ptr<Scene> scene = m_sceneObject;
return scene->update();
}
You probably won't need the SceneID enumeration at all, and scene manager won't need to know about all the different scenes that exist at all.
You'll have to give scenes a way to tell SceneMgr to change the scene -- you could pass each scene a reference to SceneManager (but that's kinda gross), you could pass a call-back function that changes it (which is better), or you could re-jigger things so that Scene::Update returns the next scene, or itself if its to stay in the same state -- shared_ptr should take care of all the reference count management automatically. A scene that kicks of any new scenes will have to know about those scenes and those scenes only, which is the way it should be.
That's definitely a clever use of a shared_ptr.
The original implementation of this system, long ago, used a similar method, with each scene returning either NULL, this, or a new scene object. I eventually migrated to this method to avoid certain potential resource conflicts and to make it easier to create dynamic scene transitions. I found that within a Scene::update() there were times when I wanted to perform certain actions regardless of whether the scene was going to start transition at the end of the frame. Also that method requires all Scene derivatives to include the header (or a class prototype or whatever its called) of all other Scene derivatives in order to instantiate the new object. I don't want to have to modify existing Scenes when I add new ones just to handle all possible transitions. Or, put another way, I can finish a scene and then make new ones and if I want a new transition all I have to do is change a variable value in the old one. Everything else is already in place.
There have also been some cases historically where I've wanted to trigger unique behavior with no active Scene object. The switch in SceneMgr provides a convenient place to do that, although I don't expect that circumstance to come up in Windows since there's nothing crazy to implement that can't be wrapped comfortably in a Scene.
The SceneMgr looks complicated at first but it's actually nearly the same system as what you're talking about. It's a lot smoother for me conceptually to have only one active Scene at any point in time. I'm really not worried about SceneMgr. I'm just peeved that I can't declare a virtual static const to make this SO much easier, but Álvaro dropped the 'why didn't I think of that' bomb on the only practical issues. The only remaining issue for me is aesthetic. (IOW - the real problem is solved. I'm just bitching about C++ now.)
The original implementation of this system, long ago, used a similar method, with each scene returning either NULL, this, or a new scene object. I eventually migrated to this method to avoid certain potential resource conflicts and to make it easier to create dynamic scene transitions. I found that within a Scene::update() there were times when I wanted to perform certain actions regardless of whether the scene was going to start transition at the end of the frame. Also that method requires all Scene derivatives to include the header (or a class prototype or whatever its called) of all other Scene derivatives in order to instantiate the new object. I don't want to have to modify existing Scenes when I add new ones just to handle all possible transitions. Or, put another way, I can finish a scene and then make new ones and if I want a new transition all I have to do is change a variable value in the old one. Everything else is already in place.
There have also been some cases historically where I've wanted to trigger unique behavior with no active Scene object. The switch in SceneMgr provides a convenient place to do that, although I don't expect that circumstance to come up in Windows since there's nothing crazy to implement that can't be wrapped comfortably in a Scene.
The SceneMgr looks complicated at first but it's actually nearly the same system as what you're talking about. It's a lot smoother for me conceptually to have only one active Scene at any point in time. I'm really not worried about SceneMgr. I'm just peeved that I can't declare a virtual static const to make this SO much easier, but Álvaro dropped the 'why didn't I think of that' bomb on the only practical issues. The only remaining issue for me is aesthetic. (IOW - the real problem is solved. I'm just bitching about C++ now.)
What's worrisome to me is all the places you have to include headers and such where you shouldn't, and all the upheaval that causes in your build each time a state is added. It may not be a practical problem if the number of states is small and relatively constant, but its definitely a code-smell, and it sounds like a maintenance nightmare to me. Given the current design, that doesn't look to be a problem that's going away unless you can eliminate the need for the type token.
The kinds of resource conflicts you cite could probably be handled by some kind of ref-counted resource tracker, and if unique behavior has to be triggered without a scene object, there are probably better places to do so than in the SceneManager::update function. Each such use sounds to me like a kind of "phantom state" which might or might not quite fit the usual model of a scene -- which goes to my point about how the token/switch might be being used to subvert LSP (which, does apply here; virtual base classes and their derivatives are not exempt -- an interface is an interface is an interface).
My use of shared_ptr might be astute, but its not clever -- all it's saying is that during update SceneMgr is sharing ownership of m_gameState with SceneMgr::update, which is true because they both have a vested interest in the state, just with different purposes in mind.
The kinds of resource conflicts you cite could probably be handled by some kind of ref-counted resource tracker, and if unique behavior has to be triggered without a scene object, there are probably better places to do so than in the SceneManager::update function. Each such use sounds to me like a kind of "phantom state" which might or might not quite fit the usual model of a scene -- which goes to my point about how the token/switch might be being used to subvert LSP (which, does apply here; virtual base classes and their derivatives are not exempt -- an interface is an interface is an interface).
My use of shared_ptr might be astute, but its not clever -- all it's saying is that during update SceneMgr is sharing ownership of m_gameState with SceneMgr::update, which is true because they both have a vested interest in the state, just with different purposes in mind.
The unique case I had in mind was one where I had to free almost all the memory in use to trigger a resource intensive system call that also took control of the device's audio and video for the duration of its execution. That platform had a couple cases like that.
In the current implementation I only have to include the header for a new scene in ns_SceneMgr.cpp, which instantiates the scene objects. If I changed it I'd have to add each new scene header to all the existing scenes and add all of them to the new scene.
The current imp doesn't really have a lot of include headers. (?) I don't understand what you're worried about there. The vclass includes the SceneMgr header and the SceneMgr code file includes its own header, the vclass header (which is redundant but explicit) and its inheritors. Apart from that the inclusions are unrelated.
Changing or adding a Scene derivative is a relatively quick rebuild. If you're concerned about the change of the enum triggering a recompilation of all scenes then I'd submit that this would happen anyway without the enum since, once again, I'd have to add headers between all the scenes. It would just mean that I'd have to do more footwork to accomplish the same result.
In the current implementation I only have to include the header for a new scene in ns_SceneMgr.cpp, which instantiates the scene objects. If I changed it I'd have to add each new scene header to all the existing scenes and add all of them to the new scene.
The current imp doesn't really have a lot of include headers. (?) I don't understand what you're worried about there. The vclass includes the SceneMgr header and the SceneMgr code file includes its own header, the vclass header (which is redundant but explicit) and its inheritors. Apart from that the inclusions are unrelated.
Changing or adding a Scene derivative is a relatively quick rebuild. If you're concerned about the change of the enum triggering a recompilation of all scenes then I'd submit that this would happen anyway without the enum since, once again, I'd have to add headers between all the scenes. It would just mean that I'd have to do more footwork to accomplish the same result.
meh... as usual ppl are looking for language constructs to fix their own deeply flawed design.
The only place I can see this "ident" used is here:
if(sceneID != g_sceneObject->ident)
And this has nothing to do with polymorphism . The coupling between an enum and a type only lives inside the SceneManager , it is something that just makes sense in the scene manager world... so why propagate this nonsense outside the SceneManager?
The line above is nonsense.. starting from the names. "sceneID" doesn't really describe what that value is.. what it really is is a "requestedSceneID".. so probably the SceneManager should have also a "currentSceneID" as member.. so the test would be a more sane:
if (requestedSceneID!=currentSceneID)
{
// it's time to change scene a create a type matching the requestedSceneID
// And once I am done with it
currentSceneID=requestedSceneID;
}
But the design is flawed nevertheless..what if I want a new scene with the same ID as the one that was already there? The SceneManager won't even notice that.
The solution? Simple.. just add a function to call WHEN you want to change scene.. and stop all the state nonsense of setting global variables as there is no tomorrow. If I want to change scene I call:
void SetCurrentScene(eMagicSceneID id)
{
// get rid of the old scene (if there is one).. this is just a IScene*
if (currentScene) delete currentScene;
// Resolve the enum -> type (NOTE THAT THIS IS ONLY DONE HERE
switch (id)
{
case bla:
currentScene=new SceneBla();
break;
//and so on
}
Initialise, dont have to be an exception.. Initialise just calls SetCurrentScene( id of whatever initial thing you want to create). Scene management logic just lives in 1 function.
There you go.. the magic coupling is hidden inside a single and clear switch statement and classes just do what they have to do for themselves, not for a stupid SceneManager that is asking a type to remember a number to be associated with for no reason at all.
The only place I can see this "ident" used is here:
if(sceneID != g_sceneObject->ident)
And this has nothing to do with polymorphism . The coupling between an enum and a type only lives inside the SceneManager , it is something that just makes sense in the scene manager world... so why propagate this nonsense outside the SceneManager?
The line above is nonsense.. starting from the names. "sceneID" doesn't really describe what that value is.. what it really is is a "requestedSceneID".. so probably the SceneManager should have also a "currentSceneID" as member.. so the test would be a more sane:
if (requestedSceneID!=currentSceneID)
{
// it's time to change scene a create a type matching the requestedSceneID
// And once I am done with it
currentSceneID=requestedSceneID;
}
But the design is flawed nevertheless..what if I want a new scene with the same ID as the one that was already there? The SceneManager won't even notice that.
The solution? Simple.. just add a function to call WHEN you want to change scene.. and stop all the state nonsense of setting global variables as there is no tomorrow. If I want to change scene I call:
void SetCurrentScene(eMagicSceneID id)
{
// get rid of the old scene (if there is one).. this is just a IScene*
if (currentScene) delete currentScene;
// Resolve the enum -> type (NOTE THAT THIS IS ONLY DONE HERE
switch (id)
{
case bla:
currentScene=new SceneBla();
break;
//and so on
}
Initialise, dont have to be an exception.. Initialise just calls SetCurrentScene( id of whatever initial thing you want to create). Scene management logic just lives in 1 function.
There you go.. the magic coupling is hidden inside a single and clear switch statement and classes just do what they have to do for themselves, not for a stupid SceneManager that is asking a type to remember a number to be associated with for no reason at all.
Yeah, okay. This stopped being constructive a while ago, so I'm done here.
This is an acceptable implementation of dynamic dispatch. I've tried (I started out with) the other method and it was an enormous pain in the ass to maintain, so I encapsulated the behavior in a very simple and much-easier-to-maintain way. It is no longer a pain in the ass and it does what I want, so I'm going to keep using it unless someone can give me a reason that doesn't consist of "I don't like it".
Thanks for whatever constructive advice was given.
This is an acceptable implementation of dynamic dispatch. I've tried (I started out with) the other method and it was an enormous pain in the ass to maintain, so I encapsulated the behavior in a very simple and much-easier-to-maintain way. It is no longer a pain in the ass and it does what I want, so I'm going to keep using it unless someone can give me a reason that doesn't consist of "I don't like it".
Thanks for whatever constructive advice was given.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement