Private constructor" causes error in SimObject child class

#1 I keep coming back to the idea of having two classes. One, the singleton, that just does it's thing, and a second class that is a conObject to act just as an interface between the console and the real class.

Unfortunetly I see more problems than answers on that path and don't think it's a very good solution.

#2 Greetings!

I have a number of "singleton" classes I've built for Constructor -- the program, not the C++ concept :o) They're not true singletons like you're attempting to do with the private constructor, but they work well when derived from SimObject.

Here's an incomplete example from my PreferencesManager. First the header:

class PreferencesManager : public SimObject
{
   typedef SimObject Parent;

public:
   static PreferencesManager* gPreferencesManager;

   DECLARE_CONOBJECT(PreferencesManager);

   PreferencesManager();
   virtual ~PreferencesManager();
};

And the source:

PreferencesManager* PreferencesManager::gPreferencesManager = 0;

//**********************************************************************************
//*** Console Functions
//**********************************************************************************

//*** Register the Preferences Manager
ConsoleFunction(registerPreferencesManager, bool, 2, 2, "registerPreferencesManager(SimObjectID) - register the script created Preferences Manager with C++ code.")
{
   if(PreferencesManager::gPreferencesManager)
   {
	   Con::errorf("registerPreferencesManager(): Preferences Manager already registered!");
	   return false;
   }

   PreferencesManager::gPreferencesManager = dynamic_cast<PreferencesManager*>(Sim::findObject(argv[1]));
   if(PreferencesManager::gPreferencesManager)
   {
	   return true;
   }
   else
   {
	   Con::errorf("registerPreferencesManagerManager(): Could not register Preferneces Manager");
	   return false;
   }
}

//**********************************************************************************
//*** PreferencesManager Implementation
//**********************************************************************************

IMPLEMENT_CONOBJECT( PreferencesManager );

PreferencesManager::PreferencesManager()
{
}

In my case, I build the PreferencesManager class from script and then attempt to register it with the system using the above console function. For example:

//*** Create the Preferences Manager singleton
   %pm = new PreferencesManager(pref);
   %check = registerPreferencesManager(%pm.getId());

Now I can access the Preferences Manager using the object name 'pref' in script. ie:

%val = pref.getVal("something");

You could of course just build the class in C++ and devise your own registration method.

And if you really want to make sure your class is only invoked once, you could put in an assert into the constructor to test for the static member, in my case PreferencesManager* PreferencesManager::gPreferencesManager. In fact, this is a good idea to add to Constructor. Thanks! :o)

- LightWave Dave

#3 Remember, a singleton is just a OOP-oriented global namespace. You can also (Dave's way is probably better) just use... functions and globals, with prefixes on them. :)

Then you never have to worry about getting more than one, too. :)

#4 Thanks guys for the great ideas I took a break from this for a few days and checked out the TLK-1.4 and RTS-enviroment pack. their cool! And now back to Singleton's.. :)

@Ben, intresting thought - I didn't know that, but it makes sense.

@Dave I'm going to try somthing like your idea. I'm thinking maybe I can use an exception instead of a assert, and when i catch the exception (ie. the object already exists) return the address of the existing instance (and delete 'this' before leaving?).

Hopefully this will still let me create the object form script OR c++ and prevent 2 copies and avoid the hard-crash an assert would give me. It's been awhile since I used exceptions though. Does this sound right?

thanks!

#5 Ahh, I see exceptions would not work now. I guess I can basicaly use my original scheme, like yours, but make the constructor public and add in the assert. If I, or anyone else later tries calling the constructor directly, instead of using getInstance(), the assert will ensure that remains their problem.

thanks again

#6 @Dave - Please Help! I'm still banging my head on this! :-(

First how did the assert work out for you?

For right now i'm derriving directly from ConsoleObject. I'm not sure if I really want SimObject, NetObject, or somthing else, but I figured i'd get some basic functionality going and worry about details, particularly involving network-support, later.


SHORT VERSION:

I have two similar 'singleton' classes derrived from console object. It appears on torque-startup the constructor is called (directly) on all objects derrived from ConsoleObject. In my case, the constructor of one calls getInstance() on the other to get a pointer to it, which invokes the 2nd classes constructor. When ConsoleObject calls the 2nd classes constructor, the constructor has already been called and the assert fails.

I can't figure out WHY ConsoleObject is automatically calling constructors on these classes, that i'm not invoking anywhere, or how to stop it because this is exactly what i DON"T want to happen. Any Ideas?

//----------------------------------------------------------------------
@all


LONG VERSION:

I"m trying to use good coding and software-engineering practices on this project, not 'hack it together'. This is all for the next geneation ground-up rewrite of a resource I hacked together earlier called 'Enviro-Torque'. It's for managing different envrionmental effects basically. Aside from submitting it as a resource I want to use it as block of example code to help get a job!

I have a very clear idea of what/how I want to accomplish this. However I'm beginning to feel i'm going about this all wrong when it comes to interfacing my code with Torque. So maybe if I give a quick rundown of what i'm trying to do hopefully someone can point me back in the right direction.



My first class is: 'class TaskManager : public ConsoleObject'

- As the top of my class diagram, it's my overriding scheduler and has a linked-list for holding sub-modules (all derrived from abstract class TskEvent). Each subModule can be a simple event to a complex manager of it's own, for example a calendar manager, celestial manager, weather manager, story-event etc.

TaskManager uses a SimEvent to schedule TaskManager.update() to happen once a second. TaskManager::update() itterates throguh the linked list and calls mySubModule::update() on each submodule.

The only thing the subModules are inheririting from TskEvent at the moment is the pure-virtual update() function just mentioned.


My Second Class is: 'class TskCalendarManager : public TskEvent, public ConsoleObject'

- This is the first submodule. It has lots of functions and variables to implement a calendar of sorts and sense of time in the game.

Both TaskManager and TskCalendarManager, I only ever want one of each in the game which is why I was thinking singleton to begin with. Both have to interface with script because there's lots of variables (mSecsPerGameHour, mDaysInWeek, mMonthsInYear, etc) that should be user defined in the *.mis file; which is why i'm trying to derrive from ConsoleObject.

Ok, I don't know if i explained that well, but thats the general idea. I'm sure there's a similar object in Torque already I could look at but I don't know what.


Thanks for all the help, I'm really feeling lost here!!!
RavenSlay3r

#7 I'm betting here that your problem is in your getInstance() function. What technique are you using to find the existing instance? If possible, post that code segment here...should help us see what the issue is.

Also, I don't think we are aware of your thought processes for selecting ConObject as the parent...gut instinct tells me you probably wanted to use SimObject, but if you could describe why you chose ConObject we might be able to get a better feel for if SimObject might be a better call.

Final thought: can you simply change the instantiation order, since the first object currently being instantiated is calling getInstance on the second, which will create the second if it hasn't been created yet? I'm betting that later in your execution flow you are directly instantiating your second singleton, and not checking to see if it's in existence yet.

#8 Obviously just an excerpt...

class TaskManager : public ConObject {

public:
static TaskManager* getInstance();

private:
static TaskManager* sTaskManager;
}


TaskManager* TaskManager::getInstance()
{	
	if ( sTaskManager == 0 ) 
	{
		sTaskManager = new TaskManager;
	}  // end if

	return sTaskManager;
}  // end function: getInstance

I was using 'sInstance', but then changed my mind to name the variable after the class ie. 'sTaskManager'.


As to ConObject:
Basically I don't know for sure what my base-class should be. Eventually I'll want things like [Torque]network-support (which i haven't used before). For now, I just want to get some core-functionality going but i need to interface to the script-side.

Since ConObject was the root-base-class that provided that functionality (to my understanding), it seemed like the simplest place to start. In regards to SimObject I read this(below). I interpretted it to say 'you must override these 14 functions to subclass 'simobject'. At that point i decided I'd hold off doing that untill I was sure it's what I need to do.

Quote:
Subclassing
You will spend a lot of your time in Torque subclassing, or working with subclasses of, SimObject. SimObject is designed to be easy to subclass.

You should not need to override anything in a subclass except:

* The constructor/destructor.
* processArguments()
* onAdd()/onRemove()
* onGroupAdd()/onGroupRemove()
* onNameChange()
* onStaticModified()
* onDeleteNotify()
* onEditorEnable()/onEditorDisable()
* inspectPreApply()/inspectPostApply()
* things from ConsoleObject (see ConsoleObject docs for specifics)

Of course, if you know what you're doing, go nuts! But in most cases, you shouldn't need to touch things not on that list.

source: www.garagegames.com/docs/tge/engine/classSimObject.php



My current problem is that it seems a by-product of subclassing ConsoleObject (or anything derrived from it) is that the subclass is instantiatiated at start-up, by calling the constructor directly, and hence without checking or setting the 'instance' variable.

This unwanted instantiation is called from or immediatly after: line 358 of "console/consoleObject.h"

/// Wrap constructor.
   ConsoleObject* create() const { return new T; }

So even though I'm making MY calls to these classes correctly "someone else" is calling them first and FUBARing things from the word go.


Like I said, I'm in new territory here and, it's becoming clear that i'm working against Torque not with it. So hopefully it will be smooth-sailing once I get over these learning-hurdles...

#9 Do you ever actually instantiate sTaskManager anywhere using the new command?

AFAIK (I -could- be wrong here, but I don't think I am), there is nothing within ConObject that is going to cause an "auto-instantiation" like you are seeing, so all I can think of is that somewhere along the line after your first call to getInstance, you are instantiating it again.

Wait a sec...how are you calling getInstance as a member function of a class that hasn't been instantiated yet?

Could you list the entire function (or at least several lines plus and minus) on where you are calling TaskManager::getInstance()?

Regarding SimObject, that example quote is slightly misleading...it says that you should not need to reimplement anything in SimObject except (list here). That means simply that you may find the need to reimplement those functions, but nothing else that SimObject provides. It doesn't mean you must reimplement them.

One of the huge things that SimObject provides (along with smart referencing, which is huge, just not completely applicable here) is the ability to register your object with the simulation, and specifically by name. This is perfect for singleton objects, because now any object in the simulation can grab a reference to that object through the simulation management system, instead of through c++ only.

If you derive from SimObject, as part of your instantiation you can do the following:

sTaskManager->registerObject("TaskManager"); // c++ code here

This will then allow you, from anywhere that it's needed, do the following (again, c++ code here):

TaskManager *curTaskManager = dynamic_cast(Sim::findObject("TaskManager"));

I assume that you plan on accesing your object via the getInstance() call, but the way you have it set up you have to be able to have a valid TaskManager before you can get a pointer to the TaskManager...obviously a catch 22.

The other really big advantage that a SimObject inheritence gives you is the ability to use the TaskManager from script via the following:

%taskManagerObjectId = nameToId("TaskManager");

This in turn lets you have further ConsoleMethods defined for your TaskManager object so that scripters can call them.

#10 @Stephen

Real-Quick - I think your misunderstanding the 'getInstance()' function. In a real singleton the constructor is private so it can ONLY be called from getInstance(). I can't do this AND inherrit from ConsoleObject or I get a compilation error... But what about the catch 22?

By declaring a class member-function static like this:

static TaskManager* getInstance();

It can be called at anytime EXACTLY like this:

TaskManager* myHook = TaskManager::getInstance();

Even if the class has NOT been instantiated yet, therby breaking the Catch-22. Counter-intuitive, but correct.



SimObject
Thanks *Much* for the info, this is a big help. That clarification on that one line of SimObject documentation makes me feel alot better. I didn't even realize I might be reading it wrong untill my last post, and even then I couldn't be sure.

If I can use the simObject registration method, then maybe I don't need my own singleton implementation at all. Saving more headaches :-)

I'm going to fully explore SimObject in the morning, and I'll be sure to post back to say how I made out.

I'm also going to make my code doxygen ready and Coding Guidelines for Torque Game Engine complient. Then if your intrested i'd be happy to email the files so you can get a better look at what i'm doing!

Thanks for all the help!
Raven

#11 Raven,

I hadn't yet added an assert to my constructor, but did just now take a look.

I believe I've tracked down your problem with a phantom construction of your ConsoleObject after putting a break point on my own PreferencesManager SimObject constructor. It comes down to GuiControlPopUp::onAdd() in gui/editor/guiControlListPopup.cc.

The GuiControlListPopUp is set up as part of Torque Creator mod I believe, which runs after your game mod. If you're setup like I am here, you'd have initialized your singleton during the game mod phase. It is here that your singleton is created. Then comes along the Torque Creator mod and GuiControlListPopUp to mess everything else up.

It looks like GuiControlListPopUp creates every ConsoleObject to find out some information about them, and then deletes them. It's none too pretty, for sure.

One solution is to not run the Torque Creator mod, but then you'd lose the GUI and mission editors. And of course your code won't work with stock Torque.

The other solution would be to not worry about catching the multiple constructions within C++, and leave it all in script. If you use my method above of having a script-based registration function that fills in a C++ static pointer and put the duplication checks there, I think you'll cover most cases. Then in C++ you'd just call some getInstance() function that returns your singleton, or NULL otherwise.

- LightWave Dave

PS: One last minute and untested thought. What if you made your constructor 'protected' rather than 'private'? That's how the ConsoleObject constructor is defined, and ConcreteClassRep has no problem with it.

#12 Wow David...great catch. I'll make sure the folks here take a look at it!

@Raven: If your goal is to use best Torque coding practices, then inheriting from at least SimObject (NetObject if you think this object will ever be networked) is a great way to go.

Quote:
TaskManager* myHook = TaskManager::getInstance();

Good point! After 3 days of intense boot camp teaching, my brain was a bit fried.

#13 Stephen

- Haha, it happens! And C++ is so friggen overloaded, it's hard to catch everything, I still keep the books open in front of me... BTW: you have any of those boot-camps coming up in the NYC area?

I'm reading up on SimObject and NetObject now, I think using one correctly will be a big help. :-)



Dave,

That is probably what is happening.

Right now I've NOT gotten as far as actually instantiating my classes, I just want the engine to start with them compiled into the project.

The only time I'm calling either of them is when TskCalendarMangager::TskCalendarManager() calls TaskManager::getInstance(). Since I don't call TskCalendarManager() in any way shape or form, it shouldn't be running, and yet both constructors deffinitly are - before anything is even written to the console-Log.

I'm going to give sim/netObject a try but I like the idea of a protected constructor. Off-hand it may require declaring ConcreteClassRep a 'friend-class'.


Thanks both, and now back to Torque :-)

#14 Hmm, I'm reading about NetObjects on: http://www.garagegames.com/docs/tge/engine/classNetObject.php

The first codeblock on that page is as follows:

class SimpleNetObject : public NetObject
    {
    public:
      typedef NetObject Parent;
      DECLARE_CONOBJECT(SimpleNetObject);

Most of the simObject and consoleObject docs stated the typeDef must be in the private section.

Can someone please verify one way or the other and if needed put in for a fix to HEAD, as this is a dOxygen document?

#15 Raven,

Yes, I do believe you're correct with the typedef requirement of being declared 'private'. We don't want that to accidentally get passed around through inheritance.

Hopefully Stephen is still watching this thread and can get that document comment fixed for TGE 1.4.1. Otherwise I'd recommend that you toss him an email.

- LightWave Dave

#16 Dave, Good idea, I'll email him, if he doesn't post here in a day or so.


It's my current understanding that making a call like below in script will automically register the simObject with the simulation, but if i did it in C++ then i would have to call registerObject myself. Yes/No/Maybe?


IN SCRIPT:

%gTaskManager = new TaskManager(myTaskManager)
           {           
               mSecsPerGameHour = 30;
           }

Is this correct - assuming TaskManager is a derrived from SimObject, and mSecsPerGameHour is listed in addField() under initPersistFields? Then if i wanted to access this object from C++ I'd just use: %myHook = Sim::findObject('myTaskManager') I think.


Sorry for all the questions guys, I think i'm slowly getting it though.
I really do appreciate all the help!

#17 Point noted on the example code. I think you guys are the first to catch that to be honest, and it's been there for years!

@Raven: you are absolutely correct, the script new operator handles all of the SimObject requirements for you.

In the example you give above, you should note that several things happen (you probably know this, I'm just being complete for folks that may be watching the thread):

• An instantiation of the class TaskManager is created (memory space set aside, constructor called, etc.)
• The object is registered with the Simulation two ways: first, a semi-random (at least to the developer) ObjectID is assigned to the object, and since you included the (myTaskManager) portion in your useage of the new operator, the object is also registered by name.
• This object is linked to the namespace associated with the class (C++) of the object. In other words, this object now accesses the TaskManager:: script namespace.
• with your assumption that mSecsPerGameHour is linked to a class member variable via TaskManager::initPersistFields, yes this will work as well. Normal usage is to provide a more "script-friendly" link name that isn't commonly the exact same name as the member variable, but you can.
• The new operator returns the objectId assigned by the simulation, and in your example is assigned to %gTaskManager.

If you were to do this instantiation yourself, you would need to call ->registerObject() yourself.

Tracking down the object from script is slightly different: we expose a consoleFunction for you called "nameToId" that is used like this for script:

%myHook = nameToId("myTaskManager");

Obviously, this only works for named objects. What's even more interesting is that you can actually just do this since it's a named object:

myTaskManager.mSecsPerGameHour = 45;

and it will work--the console does an automatic conversion for you implicitly as part of the tokenization when it sees myTaskmanager used that way.

#18 Thanks Stephen, that helped!

Progress is slow but i'm getting closer. Since a picture's worth a 1000 words, and I finally figured out how to make one here's my guiding vision.

My UML is a little rusty so:

KEY:
solid-lines = inheritance
dotted-lines = SimGroup that the pointed to object can iterate through calling 'update()' on each object


NOTE 1: I was originally using my own linked-list to manage the groups marked with dotted-lines. Right now I have some memory-leak issues though; rather than insure objects remove themselves from the linked-list before destruction, I'm just going to use SimGroups now that I know they exist.

NOTE 2: Hopefully you can see now why I want most of the TaskEvent's to be singleton's although there's no reason one couldn't write a 'TskShootingStar' for example and have lots of them...

NOTE 3: With the celestial-objects (hopefully) multiples of each type WILL be allowed. I think 'CelestialSun' will inherit from 'Sun' and reimplement most of 'fxSunLight' along with new properties for a propper solar-orbit and day/night effects. Still have to research all that though.


BTW: If I actually get a handle on this, I'm thinking about writing a 'Beginner's Guide to SimObjects' for the TDN.

~'Nuff for Now~

#19 I would highly suggest being spring-loaded to use SimSets instead of SimGroups, unless you absolutely know that you want the extra property of a SimGroup (an object can be in one and only one SimGroup, gets removed from any previous SimGroup if added to a new SimGroup, and gets deleted when the SimGroup gets deleted).

I applaud you going to SimSet/Groups instead of linked lists...just pointing out the sometimes subtle different of a SimGroup.

#20 The decsion for groups was based entirely on the examples present in stronghold.mis. Based on your recomendation, I'll change it. Thanks.

Just by better organizing the code, converting to SimObject/SimSet use and stripping out the old code, I've solved a lot of trivial errors. I deffinitly have one or two deffinitve bugs in my code, up to this point though. I'm kind've hoping they resolve themselves as I go.

Talk about a learning project, lol!

edit


Ok I ran into a problem with using SimSets.

Coincidently I found this thread Creating a SimObject and the guy just reiterated what we came up with for a singleton SimObject. Basically putting an assert in the public constructor, and calling getInstance() and hoping for the best.

The problem with BOTH of these is they seem to prohibit me from listing the object in SimGroup(MissionGroup) in the *.mis file and hence in the F11-World Inspector Editor.

As far as I can figure, my best bet is to use SimSets, and a getInstance() and initlize these classes in a new script file(eg. environment.cs) and exec() it from game.cs. From that file I'll provide some kind of interface for the end-user to set variables (in script only, NOT in-game).

If anyone can improve on that i'm open - as always ;-)


ps. Hope you all had a good weekend :-)