Hi all. This question is somewhat linked to my problem I posted here and a singleton was suggested. Now, after a brief reading of singletons(although it was over my head), this doesn't seem to be what I want. And i'm just wasting my time here, until I say what the problem is. :P I'm not sure if this is a design flaw or just me being sill. Regardless, here is said problem. I have my main engine like so

//detects key presses :D
class KeyPress
{
public:
 bool forward;
 bool backward;
 bool left;
 bool right;
};

//This is our main game engine
class MercuryEngine
{
public:
...
       Key KeyPress;
       Sprite Player;
};

and within the Sprite class, I need to refer to the KeyPress class, specificaly, the one defined in the MercuryEngine class. How should I go about doing this? I have a bad feeling that this problem stems from a bad design, but ... well, any help would be super sweel! Cheers

If you don't want to use a singleton then you need to associate the Sprite class with either the KeyPress class or the MercuryEngine class. You can use a technique like dependency injection to get the Sprite to refer to the engine class by adding a constructor to the Sprite class:

Sprite::Sprite(MercuryEngine &engine): m_engine(engine){ // other setup }

Of course this means that you need to initialise the Sprite class when it is constructed which mean you might have trouble using Sprite as a stack allocated type and will need to change it to a Sprite pointer.

I would be tempted to have the KeyPress instance passed into the Sprite rather than have it call out to the engine so have something like this:

Sprite::ProcessKeyPress(KeyPress keyPress){ // handle key press }

Could I do somthing like this?

class Sprite(){GameEngine *engine;Sprite(MercuryEngine engine)//constructor};Sprite::Sprite(MercuryEngine Gameengine){this->engine = &Gameengine}class GameEngine{Sprite Player (this);};

Or is that just wrong?

Well, I don't think that is going to work. For starters you would want to pass the engine class by reference rather than using the reference operator on the parameter like so:

class Sprite(){GameEngine *engine;Sprite(MercuryEngine &engine)//constructor};Sprite::Sprite(MercuryEngine &Gameengine){engine = Gameengine;}

If you don't then your engine member variable will point to a location on the stack which won't be valid.

Next, you can't initialise member variables in the class declaration and have to use the initialiser clause of a constructor like so:

class GameEngine{Sprite Player;};GameEngine::GameEngine: Player(this){ // set up here }

Unfortunately, I don't think this will work as the "this" pointer isn't valid at this point (although I could have mis remembered this detail) so you need to do something else. Perhaps have a Sprite pointer?

class GameEngine{...Sprite *Player;};GameEngine::GameEngine(){   Player = new Sprite(this);   // other set up here}GameEngine::~GameEngine(){   delete Player;   // other cleanup here}

My memory is a bit sketchy about the details on "this" so it might not be valid here either but it is worth a try.

The greater question is whether you should do this at all, I think it is starting to get into the realms of tortured architecture if you have 2 classes that refer to each other. As I said before, I would be more comfortable if the Sprite class had KeyPress objects passed into it rather than have it query something else for them.

Quote:Original post by misterPhyrePhox

A good start! A couple of corrections:

class Sprite {private:  GameEngine *engine; // The pointer to the game enginepublic:  Sprite(GameEngine *_engine) { // Pass a pointer, and just set engine = _engine, easy    engine = _engine;  }};class GameEngine {private:  Sprite Player;public:  GameEngine() {    Player = Sprite(this); // Initialize Player in the ctor of GameEngine,    // pass it "this," a _pointer_ to the calling class, not an instance  }};

I don't want to be a pain but the line

Player = Sprite(this);

is only going to work if Sprite also has a default constructor that takes no arguments. Right now it doesn't so this isn't going to work. However if it did then there would have to be code to deal with a Sprite with an uninitialise pointer to a GameEngine instance which is complicating it for no real reason.

Wow..this is nuts :P lol Hmm....wow lol.

Quote:Unfortunately, I don't think this will work as the "this" pointer isn't valid at this point

I think it will be, b/c as soon as the GameEngine class is created, the 'this' pointer becomes valid. Or not? :P I'm pretty sure it does.

Well, i think I'll deal with this after I get some sleep, and thing this through logicaly.

Quote:...just make GameEngine a singleton, rather than pass a pointer to every single sprite object. Anything in particular that you don't get about them or something I could explain?

I'll have another look at it, and see if I can sort it out. Then see if its what I want.

Is it a bad idea(other than this headache) to have to intermingled classes like this? Is this frowned upon, or just kinda confusing? :P

Thanks everyone!

//edit. So fully corrected this might work?

class Sprite {private:  GameEngine *engine; // The pointer to the game enginepublic:  Sprite(GameEngine &_engine) { engine = _engine; // Pass a pointer, and just set engine = _engine, easy  }};class GameEngine {private:  Sprite *Player;public:  GameEngine() {    Player = new Sprite(this); // Initialize Player in the ctor of GameEngine,    // pass it "this," a _pointer_ to the calling class, not an instance               }};

As I said, I can't quite remember when the this pointer because valid.

Yes I think the current code might work, although you probably want to make your Sprite constructor consistent like so:

Sprite(GameEngine *_engine){engine = _engine;}

Quote:
Is it a bad idea(other than this headache) to have to intermingled classes like this? Is this frowned upon, or just kinda confusing?

I would say that confusing should be frowned upon, anything that compromises maintenance should be frowned upon. To answer your question, anytime where you have 2 objects that refer to each other you are bound to get into trouble. I am sure there are expections (there always are) but I think this is something worth avoiding if possible.

Hm...I just couldn't think of any other way around it, unless I made sprite not a class...But I don't think this current case will make maintenance any harder tho..

Well. I guess i'll try this tomorrow, and watch the fireworks :P thanks guys, i'll let you know what happens.

Quote:Original post by _Sigma
Hm...I just couldn't think of any other way around it, unless I made sprite not a class

No don't do that, keep it as a seperate class. Perhaps if you can be a bit more expansive about the problems you are facing I might be able to give you some better (and hopefully more detailed) advice. Right now I can't see what problem you are trying to solve.