21 replies to this topic

[Khatharr]

It frustrates me that I can't declare virtual data members, but anyway...

I have a situation where I have a set of classes that all derive from a virtual base and are used polymorphically. Each derived class matches a specific enum value that I use to control swapping active objects. The thing that's bothering me is that, at present, I have to set the inherited public 'ident' member (of the enumerated type) of each subclass in the constructor. I'd much rather have it as a static const for the class, but in that case I can't use the member polymorphically.

Apart from the disgusting C++ RTTI is there some way to do something like this or am I stuck with the ctor for now?

[Álvaro]

Can you do it with a virtual function that returns `ident'?

[Khatharr]

Good suggestion. That could definitely make it const, and I'll do that for now, but it would still require setting up a function. I'm wondering if there's some identifiable attribute of a class prior to compile time. Something I could set the enum values by.

Actually, I should probably just think about it more for a while. TBH what I'm thinking of probably just plain isn't there.

Edited by Khatharr, 27 November 2012 - 08:11 PM.

[SiCrane]

If you don't mind on relying on compiler specific behavior, it is possible, though extremely ugly, to embed data directly in the vtable with some C++ compilers. For MSVC, here's something I wrote when I was really, really bored that looks at single dispatch including with various type code techniques. For the bit about embedding data in the vtable skip down to non-standard approaches.

[Álvaro]

May I ask why you need to know what the actual type of the object is? If you are going to behave differently depending on the type, that should probably be encoded as a virtual function.

I subscribe to a view that is perhaps a strong version of the Liskov substitution principle: Outside of a factory function, the code should not know about the subtypes of a base class. All you need to do is have virtual functions that describe the entire interface.

[Khatharr]

The classes in question are scenes. They change a value in a namespace to transfer control. When the namespace's 'update()' function is invoked it checks to see whether its SceneID value matches the one reported by the 'Scene' object it owns. If so it invokes the object's update(). If not then it destroys the object and selects an action based on a switch of the SceneID value (usually spawning a new Scene object).

I bookmarked and will read a link I saw in another thread where someone mentioned state management without a singleton (the namespace in this case is acting like a singleton), so maybe it will become moot.

The primary things I was worried about were the non-const-ness of the Scene::ident member and also the fact that I may forget to initialize it in the ctor, resulting in an annoying error. Álvaro's earlier suggestion solves both those problems. Now it's just the asthetic of it that bothers me. (Why I can't declare a virtual static const data member is beyond me. It doesn't seem like something would be crazy to include in the language.)

I'll definitely check out SiCrane's links there. I don't want to go compiler dependant but that does sound like the kind of extreme-geek stuff I really like to mess with, lol.

[Ravyne]

First, I agree with Alvaro -- If you're using inheritance correctly, you should strive to achieve the kind of type transparency he describes. Often times, when you need to know the type of of a derived class from the outside, its because you aren't providing complete functionality via the class interface, or you're violating LSP in some way that you're compensating for.

Second, what is so disgusting about the built-in RTTI facilities compared to this method? You're essentially re-inventing RTTI on your own, I'd stick to the built in unless your platform doesn't support it, you have a good reason to avoid it, or realistic benchmarks show that its a bottleneck.

[Khatharr]

Firstly, I am using inheritance correctly. The base class is virtual so LSP doesn't really apply, and since I'm storing the objects by means of a base-class pointer they're obviously using an entirely identical interface.

Secondly, C++ RTTI is flat out disgusting. Just look at std::type_id. I'd much rather just live with the virtual function returning the class' identifying enum value than get involved in that mess. It's not a question of performance. It's a question of not throwing up every time I want to change scenes.

Edited by Khatharr, 27 November 2012 - 10:00 PM.

[Bregma]

Firstly, I am using inheritance correctly. The base class is virtual so LSP doesn't really apply

Hmm. I am having an awful lot of trouble getting those three statements to align correctly.

[Khatharr]

Firstly, I am using inheritance correctly. The base class is virtual so LSP doesn't really apply

Hmm. I am having an awful lot of trouble getting those three statements to align correctly.

Substitutability is a principle in object-oriented programming. It states that, in a computer program, if S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e., objects of type S may be substituted for objects of type T) without altering any of the desirable properties of that program (correctness, task performed, etc.).

A virtual class cannot be instantiated.
[source lang="cpp"]class Scene {public: virtual ~Scene() {}; virtual int initialize() = 0; virtual int update() = 0; SceneID ident;};[/source]
If I'm gonna use a derived class from that then it's going to have different functionality.

LSP applies in situations where the base actually has functionality. It requires that inheritors correctly implement the appropriate behavior as the base would.

Edited by Khatharr, 27 November 2012 - 10:25 PM.

[Ravyne]

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.

[Khatharr]

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 inclusionsusing 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.)

Edited by Khatharr, 27 November 2012 - 10:35 PM.

[ApochPiQ]

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?

[Khatharr]

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?

[Ravyne]

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.

Edited by Ravyne, 28 November 2012 - 12:15 AM.

[Khatharr]

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.)

Edited by Khatharr, 28 November 2012 - 12:47 AM.

[Ravyne]

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.

[Khatharr]

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.

Edited by Khatharr, 28 November 2012 - 01:37 AM.

[kunos]

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.

[Khatharr]

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.

Edited by Khatharr, 28 November 2012 - 01:55 AM.