Sign in to follow this  
BlessmanTsanga

Singletons (class templates and pure virtual functions)

Recommended Posts

hi

Im trying to create a singleton but visual studio is rejecting my private constructor.
Why is this? aren't singleton constructors meant to be private or is it that they are shunned by visual studio?

[code]class CStart_Menu :
public CStates<CGAME_MANAGER>
{
private:

CStart_Menu();

public:
//BEGIN STATE:
virtual FAST_BOOL Enter(CGAME_MANAGER* qp_laststate)
[/code]

[output]1>Start_Menu.obj : error LNK2019: unresolved external symbol "private: __thiscall
CStart_Menu::CStart_Menu(void)" (??0CStart_Menu@@AAE@XZ) referenced in function
"public: static class CStart_Menu * __cdecl CStart_Menu::Instance(void)" (?
Instance@CStart_Menu@@SAPAV1@XZ)

1>Start_Menu.obj : error LNK2019: unresolved external symbol "public: virtual __thiscall
CStates<class CGAME_MANAGER>::~CStates<class CGAME_MANAGER>(void)" (??1?
$CStates@VCGAME_MANAGER@@@@UAE@XZ) referenced in function "public: virtual __thiscall
CStart_Menu::~CStart_Menu(void)" (??1CStart_Menu@@UAE@XZ)

1>c:\users\portsmouthuni\documents\visual studio
2010\Projects\consolegamestate\Debug\consolegamestate.exe : fatal error LNK1120: 2
unresolved externals

========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
[/output]

Share this post


Link to post
Share on other sites
[quote name='SiCrane' timestamp='1318800013' post='4873222']
It's complaining that it can't find the function bodies for the functions. You might want to implement them.
[/quote]

I've done that yet its till there. its been 5 hours now and i'm seriously behind

Share this post


Link to post
Share on other sites
this is the instance I want my singletons to be controlled by

[code]
#pragma once
#ifndef _GAME_MANAGER_ZUMA_H
#define _GAME_MANAGER_ZUMA_H
#include "StateMachine.h"
//*********************************************************************************************
//Purpose: Managing the game state
//Type:
//
//*********************************************************************************************
class CGAME_MANAGER{
private:
//use our state machine
CStateMachine<CGAME_MANAGER> * qmpStateMachine;

public:
CGAME_MANAGER();


~CGAME_MANAGER(void);

//add class methods here...
void Update(void);

CStateMachine<CGAME_MANAGER>* Use_StateMachine();

};

#endif[/code]


Basically here is where I create my state;
[code]
#pragma once
#ifndef _MY_STATES_MENU_H
#define _MY_STATES_MENU_H
#include "states.h"
#include "GAME_MANAGER.h"
/****************************************************
//Purpose: This is the menu at the start of the game
*****************************************************/
class CStart_Menu :
public CStates<CGAME_MANAGER>
{
private:
//CStart_Menu(void);

public:
//BEGIN STATE:
//take message from previus state
//to see if transisation is possible
// return 1 for success, 0 for fail
virtual FAST_BOOL Enter(CGAME_MANAGER* qp_laststate);


//EXECUTE OUR STATE:
//Now lets get things rolling
virtual void Execute(CGAME_MANAGER* q);

//EXITING STATE:
//Pass message to our successor
//return our address to let the successor know,
//this message affects the call of action of our successor
virtual CStates<CGAME_MANAGER>* Exit(CGAME_MANAGER* q);
//SINGLETON DECLARATION
CStart_Menu* Instance();


//virtual ~CStart_Menu(void);
};

#define stSTART_MENU CStart_Menu::Instance()

#endif[/code]

And finally here is where it rejects my singleton :([img]http://public.gamedev.net/public/style_emoticons/default/sad.gif[/img][img]http://public.gamedev.net/public/style_emoticons/default/sad.gif[/img]:(
[code]
#include "GAME_MANAGER.h"
#include "Start_Menu.h"

CGAME_MANAGER qGameManager;

int main()
{
qGameManager.Use_StateMachine()/*->ChangingState(stSTART_MENU)*/;

qGameManager.~CGAME_MANAGER();
return 0;
}[/code]

Share this post


Link to post
Share on other sites
There's a distinct lack of function definitions in the code you just posted. The function definitions are the parts with the curly braces that have statements inside that determine what the functions actually do. Your main() function has a definition, but nothing else seems to.

Share this post


Link to post
Share on other sites
Do you know the difference between a declaration and a definition? If not, now would be the time to learn it via Google.

As a side note, any time you need to use singletons, you have done something wrong. There is always a better choice.
Even Morgan Stanley and Square Enix agree.


L. Spiro


[EDIT]
Would the person who provided me with this lovely red -1 explain whether it was due to my suggestion regarding Google or because of a personal opinion that there is some justification to use a singleton?

If you disagree with something, you should explain why.
[/EDIT]

Share this post


Link to post
Share on other sites
Prove it. Show us the .CPP files.

Because right now there are only 2 possibilities.
#1: Either you didn’t write the functions.
#2: Or you are using static libraries and are not linking to them from the main project.


L. Spiro

Share this post


Link to post
Share on other sites
Do you have function [u]bodies[/u] for CStart_Menu::CStart_Menu() and CStates::~CStates() ?

Note that you should not be explicitly calling a destructor, unless you are 200% sure of what you are doing. The compiler will emit a call to the destructor after main(), so you've actually asked to run the destructor twice there.

Share this post


Link to post
Share on other sites
[quote name='YogurtEmperor' timestamp='1318806503' post='4873254']
As a side note, any time you need to use singletons, you have done something wrong. There is always a better choice.
[/quote]

That's quite a popular and heated topic. I personaly don't see anything wrong on singletons if [b]used with care and in the right moments[/b]. What's your better choice for something like a log manager (which must be always only one) and mainly - is usually accessed from many different modules of the application? The concerns regarding creation/destruction of the singleton instance can be solved by not creating it automatically on the first call to the "GetInstance" method, but requiring an explicit construction via a special method called during the application startup.
I used to avoid singletons in the past, but now I started to use them. I find it better (again - if you know what you are doing) than a global variable or having to pass and store the instance everywhere. Ogre3D is virtually built on singletons, although I would never use them SO much, they are overusing them IMHO.

(Btw, it wasn't me who gave you -1, I always respect different opinions.)

Share this post


Link to post
Share on other sites
[quote name='Tom KQT' timestamp='1318848550' post='4873415']
I used to avoid singletons in the past, but now I started to use them. I find it better (again - if you know what you are doing) than a global variable[/quote]

But... they ARE just global variables.. but worse as they are HIDDEN variables with HIDDEN state.

If I REALLY had to go 'global' with something then it would be just that; a global.

Singletons are globals for people who want to feel warm and fluffy about being 'OO'...

Share this post


Link to post
Share on other sites
[quote name='phantom' timestamp='1318850269' post='4873418']
[quote name='Tom KQT' timestamp='1318848550' post='4873415']
I used to avoid singletons in the past, but now I started to use them. I find it better (again - if you know what you are doing) than a global variable[/quote]

But... they ARE just global variables.. but worse as they are HIDDEN variables with HIDDEN state.

If I REALLY had to go 'global' with something then it would be just that; a global.

Singletons are globals for people who want to feel warm and fluffy about being 'OO'...
[/quote]

I know what you mean, a lot of people really think that singletons are cool and that they will look cool if they are using them. But they usually start with them very early. I used my first singleton 2 months ago after cca 8 years of C++ programming and after like 15 years of programming in general, so I knew what I was doing and definitely didn't make it to look cool.
And why not a global variable? As you said, it in fact IS a global variable, so there's not so huge difference. But to make it accessible in a module, you just include a header file, you don't have to use "extern". Yep, not a huge problem, but I don't think using singleton is a huge problem either.
Of course, I don't NEED them. I know how to avoid them (as I said, I wasn't using them for 99 % of my programming history), I know their potential problems and thus, I think I can use them just fine when I'm working on a project alone.

But I agree that they are misused by many people. Global variables are misused too of course, but singletons probably even more as they are some kind of a global variable with some added features. I would probably never recommend their use to anyone :)

Share this post


Link to post
Share on other sites
I am the author of [url="http://memoryhacking.com/"]Memory Hacking Software[/url] so let me present my unique perspective, which I never see discussed when people talk about this subject.
Standard globals, static members, globals hidden behind a singleton interface—they all become the same thing in the end: A static address.
Every time the game is started, the address of that variable will be the same, unless of course the .EXE has a .reloc section, which is next to never (still have never seen one, though I could make one if I wanted).

So aside from that 0.001% of executables that allow loading to different addresses, anything you make global (which includes static class members) has a constant address that hackers can exploit most easily.
By “most easily” I mean they are the easiest thing to exploit. They are the starting point for all beginning hackers.

I recently saw a thread on here in which a guy had found the static address of a pointer to a Direct3D device context and was using it to change textures in the game and add an overlay.

Static addresses are the most essential thing hackers need to do what they do. They are security risks. Most people don’t think about this side of the coin, or think it is not a major issue. I have spent 7 years making a tool that specifically exploits static addresses, and if you plan to actually release your product and expect it to get a fair amount of attention (I assume we all expect that or else we are believing everything we make is crap) then you can expect hackers. And they are likely to be using my software to do it.
And 99.0% of the time it was all due to a few global variables that programmers inexperienced in game security left behind.


In my engine, and the games built on top of it, a pointer to the main game class is the highest security risk. From the game class pointer, you can easily get pointers to the scene and all of the objects in it, and make an auto-aim fairly easily.
Therefore the game class is actually on the stack.
The game class is not huge so it does not consume a lot of stack space, plus some consoles/handhelds give stack RAM the highest access speed (Nintendo DS, for example).


[/GameSecurity101]


L. Spiro

Share this post


Link to post
Share on other sites
the cpp files are below
The other class SateMachine is defined only, in a header file due to it having a template (I learnt this through an error which means that classes with templates can only be defined in header files)


the game manager .cpp file

[code]
#include "GAME_MANAGER.h"

CGAME_MANAGER::CGAME_MANAGER()
{ //initialise statemachine
qmpStateMachine = new CStateMachine<CGAME_MANAGER>(this);
}


CGAME_MANAGER::~CGAME_MANAGER(void)
{
delete qmpStateMachine;
}

//add class methods here...
void CGAME_MANAGER::Update(void)
{ //run current state
qmpStateMachine->Update();
}

CStateMachine<CGAME_MANAGER>* CGAME_MANAGER::Use_StateMachine(){
return qmpStateMachine;}[/code]


here is the Start_Menu.cpp file
[code]
#include "Start_Menu.h"

CStart_Menu::CStart_Menu(void)
{//must define constructor
}

//BEGIN STATE:
//take message from previus state
//to see if transisation is possible
// return 1 for success, 0 for fail
FAST_BOOL CStart_Menu::Enter(CGAME_MANAGER* qp_laststate)
{ //Test if there is a transisation
//from the previous state
qp_laststate->Use_StateMachine()->CurrentState();
/*
if ((qp_laststate->Use_StateMachine()->CurrentState())== (extern)knownstate)
if(!(execute trasition()))//returns true if transition complete
{return 0}//else carry on with transition
else if (try known transition number 2)
.....

else
//ADD ANIMATION:

*/
//LISTEN FOR BUTTONS (skip option here)


//transition complete
return 1;
}


//EXECUTE OUR STATE:
//Now lets get things rolling
void CStart_Menu::Execute(CGAME_MANAGER* q)
{
}

//EXITING STATE:
//Pass message to our successor
//return our address to let the successor know,
//this message affects the call of action of our successor
CStates<CGAME_MANAGER>* CStart_Menu::Exit(CGAME_MANAGER* q)
{
if (CStart_Menu::Instance())
return CStart_Menu::Instance();
else
return NULL;
}



CStart_Menu::~CStart_Menu(void)
{
}

//SINGLETON DECLARATION
CStart_Menu* CStart_Menu::Instance()
{
static CStart_Menu pInstance;
//this is where the instace declare it self
//it must be in a cpp (not header) file to stop
//multi declaration
return &pInstance;
}
[/code]

Share this post


Link to post
Share on other sites
here is the StateMachine file if you wanted to see (this the one I define function in .cpp files, since its forgets that my [code]template <class qXType_state>[/code])
[code]#pragma once
#ifndef _MY_STATES_MACHINE_H
#define _MY_STATES_MACHINE_H
#include <Windows.h>
#include "States.h"
template <class qXType_state>
class CStateMachine
{
private:
//This pointer points to the "owner" of this instance
qXType_state* qmp_Owner;
//the current state being used
CStates<qXType_state>* qmp_CurrentState;
//a record of the last state used
CStates<qXType_state>* qmp_PreviousState;

public:

//constructor
CStateMachine(qXType_state * owner)
{
qmp_Owner = owner;
qmp_CurrentState = NULL;
qmp_PreviousState = NULL;
}

//RUN OUR CURRENT STATE
void Update()const {
qmp_CurrentState->Execute(qmp_Owner);
}

//STATE TRANSITION TO NEW STATE:
FAST_BOOL ChangingState(CStates<qXType_state>* qNewState)
{
return (FAST_BOOL)0;
}

//set current state
void SetCurrentState(CStates<qXType_state>* qp_st)
{
qmp_CurrentState = qp_st;
}

//return to the previous sate
void RevertToPreviousState()
{
ChangingState(qmp_PreviousState);
}

//Read current state
CStates<qXType_state>* CurrentState() const
{
return qmp_CurrentState;
}
CStates<qXType_state>* PreviousState() const
{
return qmp_PreviousState;
}

//check whether NEW_STATE == CURRENT_STATE (address comparison)
//TRUE: return 1
//FALSE: return 0
FAST_BOOL State_Active(CStates<qXType_state>* st)const
{
if (st == qmp_CurrentState)
{return (FAST_BOOL)1;}
else
{return (FAST_BOOL)0;}
}
~CStateMachine(void)
{
}

};
#endif [/code]

Share this post


Link to post
Share on other sites
FYI, putting void where your function parameters is unnecessary in C++, and often discouraged. Also, anything starting with an underscore, followed by an uppercase, is reserved by the C++ standard for the compiler (meaning that _GAME_MANAGER_ZUMA_H is breaking this). Basically it means you're not supposed to do it, because names like those (or names starting with two underscores) are reserved for the compiler, not you. It may not be a problem right now, but it my bite you in the butt some day, so it's best to just do it right and not do it at all. Also, inclusion guards and a #pragma once are redundant. Any compiler that implements #pragma once will do it properly. And any compiler that doesn't will choke on the code anyway because there's a #pragma once.

Back to what rip-off said... What about your templated CStates class? One of the things it's complaining about there is the fact that it's destructor is not defined. Can you show us the code for the CStates class?

Also, is that the full output from the compiler? Are there any other warnings or errors?

Share this post


Link to post
Share on other sites
I don't see the CStates destructor in that code. Note that as CStates is a template, the definition must be available when the template is instantiated. In practise, the typical solution is to implement the functions in the template class body, or in the header file of the template. A more complex solution is to create a separate file with the template implementation in it, and include that into the header.

The CStart_Menu constructor appears to be defined in that latest post. Is the compiler still issuing errors about this function? Sometimes the compiler can get confused about which code needs to be recompiled. Do a clean build and check the error messages it is currently generating.

There are two main problems hindering us from helping you at the moment.

One is that the code you've posted so far isn't cohesive or comprehensive. It spans a period of time, in which (I assume) you've been making changes to try and fix this problem. As a result, the latest code you've posted might not correspond directly with the earlier code and error messages you were getting. It really is hard for us to decipher what is going on without a consistent set of headers, source files and compiler error messages at a particular point in time.

This is exacerbated by the second problem, which is that you haven't really made an attempt to produce a minimal example of the behaviour. That is, your code still contains lots of references to other things which aren't related to the errors.

I would recommend making a copy of your project, and progressively stripping out functionality while retaining the same error messages. Remove unrelated files from the project. Comment out any members and functions that aren't related to the problem. Remove empty constructors and destructors (except where necessary for virtual destruction). Replace FAST_BOOL with regular C++ types (what is a FAST_BOOL anyway?).

Eventually, you should be able to get it down to a handful of short headers, source files and a simple main() function. You might solve the problem by following these steps, because the total amount of code then should be small enough to manage. If not, it will be much easier for you to post it here for us to help you, and we can try compile the code on our own machines if necessary.

Share this post


Link to post
Share on other sites
At this point there are just too many questions to be asked and I don’t even feel you can answer them.
It took 1.5 days just for you to post the .CPP files. We requested them so many times yet it took over a day for you to post them.

So why don’t you just upload your project somewhere?

I personally am not going to continue the guessing game. By the way, did you rebuild all?


L. Spiro

Share this post


Link to post
Share on other sites
Building your project, I get the following errors:
[quote]
1>------ Rebuild All started: Project: Improved GameState no singleton, Configuration: Debug Win32 ------
1> stdafx.cpp
1> Start_Menu.cpp
1> Improved GameState no singleton.cpp
1> GameStateManager.cpp
1> Generating Code...
1>Start_Menu.obj : error LNK2019: unresolved external symbol "public: __thiscall [b]CStates[/b]<class CGameStateManager>::[b]CStates[/b]<class CGameStateManager>[b](void)[/b]" (??0?$CStates@VCGameStateManager@@@@QAE@XZ) referenced in function "public: __thiscall CStart_Menu::CStart_Menu(void)" (??0CStart_Menu@@QAE@XZ)
1>Start_Menu.obj : error LNK2019: unresolved external symbol "public: __thiscall [b]CStates[/b]<class CGameStateManager>::[b]~CStates[/b]<class CGameStateManager>[b](void)[/b]" (??1?$CStates@VCGameStateManager@@@@QAE@XZ) referenced in function "public: __thiscall CStart_Menu::~CStart_Menu(void)" (??1CStart_Menu@@QAE@XZ)
1>C:\...\Improved GameState no singleton\Improved GameState no singleton\Debug\Improved GameState no singleton.exe : fatal error LNK1120: 2 unresolved externals
========== Rebuild All: 0 succeeded, 1 failed, 0 skipped ==========
[/quote]
Indeed, I cannot find implementations of the highlighted functions.

You can fix these errors by implementing these functions inline (remember, this class is a template, refer to my earlier post for more information):
[code]
#ifndef _MY_STATES_H
#define _MY_STATES_H
//****************************************************
//Author: Blessing Tsanga
//Purpose: virtual parent for all states
//****************************************************
#define FAST_BOOL unsigned char
template<class xUsers_states>
class CStates
{
public:
CStates(void)
{
}

//BEGIN STATE:
//take message from previus state
virtual FAST_BOOL Enter(xUsers_states*)=0;

//EXECUTE OUR STATE:
//Now lets get things rolling
virtual void Execute(xUsers_states*)=0;

//EXITING STATE:
//Pass message to our successor message (this class id)
virtual CStates<xUsers_states>* Exit(xUsers_states*)=0;

//virtual destructors are needed for pure virtual classes
//its children will handle differently
virtual ~CStates(void)
{
}
};

#endif
[/code]
Note that I needed to add the virtual keyword to your destructor. Despite the comment, your destructor was not virtual.

Changing that file, results in the following compiler output:
[quote]
1>------ Rebuild All started: Project: Improved GameState no singleton, Configuration: Debug Win32 ------
1> stdafx.cpp
1> Start_Menu.cpp
1> Improved GameState no singleton.cpp
1> GameStateManager.cpp
1> Generating Code...
1> Improved GameState no singleton.vcxproj -> C:\...\Improved GameState no singleton\Improved GameState no singleton\Debug\Improved GameState no singleton.exe
========== Rebuild All: 1 succeeded, 0 failed, 0 skipped ==========
[/quote]

You can also remove the constructor entirely (it does nothing) instead of implementing it.

I question the need for the FAST_BOOL type. If you want to use it, at least use a typedef rather than abuse the preprocessor. But I suspect that it isn't any "faster", and only complicates your code.

Share this post


Link to post
Share on other sites
I had been taking your recommendations, with a being one being to strip down the project, rewrite it in a console window, as you saw. But now I've reached my tether's end and can't not working on it unless somebody can show me the possibly tiny but significant error in it.

Plus having talked to my lecturer, I've been told to give up on it and rewrite it in a simpler way (no templates or singletons) with major criticism being that it was wasting my time. (& sadly im 4 days behind schedule)

Share this post


Link to post
Share on other sites
[quote]
But now I've reached my tether's end and can't not working on it unless somebody can show me the possibly tiny but significant error in it.
[/quote]
I believe I have highlighted your error, and suggested a fix.

[quote]
Plus having talked to my lecturer, I've been told to give up on it and rewrite it in a simpler way (no templates or singletons) with major criticism being that it was wasting my time. (& sadly im 4 days behind schedule)
[/quote]
This is very good advice. When you have some spare time, you should consider coming back to this and solving this problem at some stage.

Share this post


Link to post
Share on other sites
I agree with the above but I want to point out that this is the type of error that can bite you in the ass no matter what the complexity of your project is, unless you specifically understand it and know how to fix it.
We asked many times for evidence of a “definition” of those functions, and when the project was delivered it was finally discovered that, despite claims to the contrary, there were no “definitions”, only “declarations”.

As I said in my first reply, you need to understand what these terms mean. I was not joking.

For the full details, ask your lecturer, as he or she will be able to answer your questions in real-time as you have them and provide you with sufficient examples.
But the skinny is this:

void MyFunc(); <- It has a semi-colon. It is a “declaration”. Goes in the .H file.

void MyFunc() { <- Braces. This is the “definition”. Typically goes in the .CPP file, but sometimes in the .H.
}


Your problem was caused by the linker. It saw the declaration in your .H file. The declaration simply says that the function exists, somewhere. That’s good enough for the compiler, but the linker has to actually track down the actual function in order to use it. Without the function being “defined”, the linker can’t do this. This is the cause of your errors, and you need to remember this for every project you continue to write, big or small.


L. Spiro

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this