For example,what I want to do is access getter from current class:

class Player
{
private:

  Vector3 pos;    //Position

public:
    Player(char *filename);
    ~Player();

    Vector3 GetPos() { return pos; }
    Vector3 GetForward() { return forward; }
};

and access it from here

void CameraWorks::Update()
{
        pos =/*player pos + etc2*/;
}

.....so what I want to know is which is the better way to get access to other class function,I know of 3(2?) ways to do it:

1.  Add   extern Player*player; in player.h and use player->GetPos(); ----->what I'm currently using,but it doesn't sound safe

2. adding Singleton Pattern and do player->getInstance()->GetPos(); -------> this also works but i heard singletons are bad

3. do something like this:

void CameraWorks::Update(Player*player)--------------->don't know if this work tho'

Which one is better design ? or are they all bad and theres other ways to do this?

Edit:

Code above is incomplete one,my current camera is like this

void CameraWorks::Update()
{
    type = player->GetCurState();    

    switch (type)
    {
    case camType::DEFAULT:
        pos = player->GetPlayerPos() - player->GetPlayerForward() * 5 + Vector3(0, 5, 0);
        mViewport->Set(pos , player->GetPlayerPos() + Vector3(0,4,0));
        break;
    case camType::AIM:
        pos = player->GetPlayerPos() - player->GetPlayerForward()*4 + Vector3(1, 3.5f, 0);
        mViewport->Set(pos, player->GetPlayerPos() + Vector3(2, 3.5f, 0));
        break;
    case camType::SCRIPTED://for cutscene
        break;
    }
}

Currently having it only follow players back TPS-style

Create an instance of the Player class somewhere, and pass it to the camera. Maybe the camera has a "Player *currentPlayer" variable, which tells it which player to track. Camera could then have a "SetPlayer(Player *player)" function, which sets its "currentPlayer".

In update, you could then do "currentPlayer->GetPos();"

For this suggestion to work, you should include player.h in the camera.h file, and player.cpp in the camera.cpp file.

IMO none of those two ways are good, but of another reason than you may think: It is because all of them require the camera to know what the class Player is about. This is bad due to a couple of reasons:

a) The camera is bound to the player object and cannot be used with another object or in a cutscene.

b) All the information belonging to the player object but besides its position is granted to the camera, although being meaningless for the camera.

So what you want is that the camera can access some geometrical informations like a position, and that's it.

However, that does not answer your OP. So ... A third possibility is to not handle a camera separate from other objects that are constrained. From an architectural point of view, you have a placement (i.e. position and orientation) that a both contrained, e.g. the position by parenting and the orientation by a look-at. Parenting needs the parent object's placement as reference and a local placement as offset. Look-at needs the own placement and the target placement (just its position, to be precise). Notice that the description above does mention neither the existence of a camera nor of a player object. Instead, it mentions placements and relations between them.

When running a game loop, you may have several steps that alter placements: player controlled or AI controlled movement, animation, physics, collision correction, and - well - some constraint resolution. Running the said "constraint resolution" updates all placements that are constrained. It does this en bloc, so after doing so all placements are up-to-date. So the camera's placement is already ready to use when rendering happens later on in the game loop.

The above solution has several advantages: It concentrates the problem solution, so being better suited to handle multiple object dependencies and detecting circular dependencies. It further is generally more efficient because it is more cache friendly. At last, it handles a specific responsibility.

Thanks for the response...Firstly:

6 hours ago, Lactose said:

Create an instance of the Player class somewhere, and pass it to the camera. Maybe the camera has a "Player *currentPlayer" variable, which tells it which player to track. Camera could then have a "SetPlayer(Player *player)" function, which sets its "currentPlayer".

I'm thinking of having camera only follow players and do cutscenes,so.....I don't know if i need that(?),thanks tho.

6 hours ago, haegarr said:

IMO none of those two ways are good, but of another reason than you may think: It is because all of them require the camera to know what the class Player is about. This is bad due to a couple of reasons:

a) The camera is bound to the player object and cannot be used with another object or in a cutscene.

b) All the information belonging to the player object but besides its position is granted to the camera, although being meaningless for the camera.

So what you want is that the camera can access some geometrical informations like a position, and that's it.

this.I need an efficient way that is lighter.Tho'.....

6 hours ago, haegarr said:

However, that does not answer your OP. So ... A third possibility is to not handle a camera separate from other objects that are constrained. From an architectural point of view, you have a placement (i.e. position and orientation) that a both contrained, e.g. the position by parenting and the orientation by a look-at. Parenting needs the parent object's placement as reference and a local placement as offset. Look-at needs the own placement and the target placement (just its position, to be precise). Notice that the description above does mention neither the existence of a camera nor of a player object. Instead, it mentions placements and relations between them.

When running a game loop, you may have several steps that alter placements: player controlled or AI controlled movement, animation, physics, collision correction, and - well - some constraint resolution. Running the said "constraint resolution" updates all placements that are constrained. It does this en bloc, so after doing so all placements are up-to-date. So the camera's placement is already ready to use when rendering happens later on in the game loop.

The above solution has several advantages: It concentrates the problem solution, so being better suited to handle multiple object dependencies and detecting circular dependencies. It further is generally more efficient because it is more cache friendly. At last, it handles a specific responsibility.

I'm kinda lost here....is this about how the camera works? If so,then I'm sorry,I think I'm not specific enough...I'll edit the OP a bit 

This works:

13 hours ago, deathwingA said:

3. do something like this:

void CameraWorks::Update(Player*player)--------------->don't know if this work tho'

This ensures that CameraWorks::Update doesn't depend on global state. (Like it would in method 1 with "extern Player *player")

Having functions depend on global state is generally considered a bad idea because it makes your code less flexible and makes it harder to reason about quickly. It's less flexible because now this function __needs__ a globally defined Player object to do it's work and it becomes harder to move code around to refactor. It makes the code harder to reason about because instead of just having the keep this function in your head, you now have to know about the entire global state that it uses too. This can be hard to keep track of correctly and you can often forget things that it depends on, leading to bugs that are hard to track down. Maybe your program doesn't exhibit these problems, but I've found that they crop up as the project gets larger.

The next thing I'd say that it's a bad idea for your camera to depend on a Player object to know where it has to point. In the code you provided you're not really doing anything player specific. If I were writing it, I'd try to keep the camera from having to know about the Player class, even if that's the only thing it's used for. It de-couples your classes, keeps your code flexible and it's one less include file! I'd also move the cutscene logic to a separate function, and rename it for clarity:

void CameraWorks::FollowObject(Vector3 position, Vector3 direction, camType type)
{
	switch (type)
	{
		case camType::DEFAULT:
			pos = position - direction * 5 + Vector3(0, 5, 0);
			mViewport->Set(pos , position + Vector3(0,4,0));
			break;
		case camType::AIM:
			pos = position - direction * 4 + Vector3(1, 3.5f, 0);
			mViewport->Set(pos, position + Vector3(2, 3.5f, 0));
			break;
	}
}

// Later on...
camera.FollowObject(player->GetPlayerPos(), player->GetPlayerForward(), player->GetCurState());

2 hours ago, yyam said:

This works:

This ensures that CameraWorks::Update doesn't depend on global state. (Like it would in method 1 with "extern Player *player")

Having functions depend on global state is generally considered a bad idea because it makes your code less flexible and makes it harder to reason about quickly. It's less flexible because now this function __needs__ a globally defined Player object to do it's work and it becomes harder to move code around to refactor. It makes the code harder to reason about because instead of just having the keep this function in your head, you now have to know about the entire global state that it uses too. This can be hard to keep track of correctly and you can often forget things that it depends on, leading to bugs that are hard to track down. Maybe your program doesn't exhibit these problems, but I've found that they crop up as the project gets larger.

The next thing I'd say that it's a bad idea for your camera to depend on a Player object to know where it has to point. In the code you provided you're not really doing anything player specific. If I were writing it, I'd try to keep the camera from having to know about the Player class, even if that's the only thing it's used for. It de-couples your classes, keeps your code flexible and it's one less include file! I'd also move the cutscene logic to a separate function, and rename it for clarity:

void CameraWorks::FollowObject(Vector3 position, Vector3 direction, camType type)
{
	switch (type)
	{
		case camType::DEFAULT:
			pos = position - direction * 5 + Vector3(0, 5, 0);
			mViewport->Set(pos , position + Vector3(0,4,0));
			break;
		case camType::AIM:
			pos = position - direction * 4 + Vector3(1, 3.5f, 0);
			mViewport->Set(pos, position + Vector3(2, 3.5f, 0));
			break;
	}
}

// Later on...
camera.FollowObject(player->GetPlayerPos(), player->GetPlayerForward(), player->GetCurState());

 

Looks great,thanks....this does allow me to include less header,which is good

I dont know how your game is assembled but why not "group" the camera below the player? Using engines like Unity 3D you have a Scene Graph that keeps update tracking of your in scene placed models. This technic is used by many rendering engines out there (and game engines consists of rendering engine) so why not use something like this too? The advantage is that you have to group your camera below the player "object" as its parent so your scene graph will transit the player transform to your cameras transform while it keeps also its own transform (as offset from the players transform as origin). This results in your camera dosent need to do any update on itself using a player object and dosent even know about what a player is.

Another way could be using some kind of event that is fired when player position changes.

Another way could be an ECS where your camera might be attached to the player's transform component or vice versa

This are not C++ ways of doing what you intended to do but game development ways

16 hours ago, deathwingA said:

...

I'm kinda lost here....is this about how the camera works? If so,then I'm sorry,I think I'm not specific enough...I'll edit the OP a bit 

It's about decoupling. In the OP you've shown ways of how the camera's update may be implemented, but only if the camera has exact knowledge of how the player object works. This is a kind of coupling that should be avoided. The way I've suggested removes that coupling. Furthermore, it separates concerns: Things like animation and geometrical contraints (i.e. the things that you handle in the camera class' switch construct) are moved out of the camera's core code. 

Let's try to solve the problem in a way where parts of the monolithic class design are put into their own classes.

First we define a class that stores the position and orientation of an object in 3D space. The class allows to get and set the current state of position and orientation, e.g. so (not saying that this is a complete example or already totally fine in any way):

class Placement {

public: // settings

    Vector3 position;
    Matrix3x3 orientation;

};

We could now build a Camera class and a Player class so that they have a Placement instance as members:

class Camera {

public: // 3D stuff

    Placement placement;

public: // camera specific stuff

    ...
};


class Player {

public: // 3D stuff

    Placement placement;

public: // player specific stuff

    ...

};

Now think of a class that is concerned with e.g. aiming (or parenting, or orbiting, or tracking, or ...).

class Aiming
    : public GeometricConstraint {

public:

    const Placement* pointOfInterest;
    Placement* target;

    virtual
    void update() override;

};

Notice that this class works with placements. It does not need to know whether a placement is part of a camera, a player object, or anything else. This is true for both the placement that is altered by the automatism (i.e. the target) as well as the placement that is used as auxiliary (i.e. the pointOfInterest). 

On 10/27/2017 at 3:09 PM, Shaarigan said:

I dont know how your game is assembled but why not "group" the camera below the player? Using engines like Unity 3D you have a Scene Graph that keeps update tracking of your in scene placed models. This technic is used by many rendering engines out there (and game engines consists of rendering engine) so why not use something like this too? The advantage is that you have to group your camera below the player "object" as its parent so your scene graph will transit the player transform to your cameras transform while it keeps also its own transform (as offset from the players transform as origin). This results in your camera dosent need to do any update on itself using a player object and dosent even know about what a player is.

Another way could be using some kind of event that is fired when player position changes.

Another way could be an ECS where your camera might be attached to the player's transform component or vice versa

This are not C++ ways of doing what you intended to do but game development ways

Thinking of doing this if I decide to do seperated camera between normal one and cutscenes.Thanks tho’.

22 hours ago, haegarr said:

It's about decoupling. In the OP you've shown ways of how the camera's update may be implemented, but only if the camera has exact knowledge of how the player object works. This is a kind of coupling that should be avoided. The way I've suggested removes that coupling. Furthermore, it separates concerns: Things like animation and geometrical contraints (i.e. the things that you handle in the camera class' switch construct) are moved out of the camera's core code. 

Let's try to solve the problem in a way where parts of the monolithic class design are put into their own classes.

First we define a class that stores the position and orientation of an object in 3D space. The class allows to get and set the current state of position and orientation, e.g. so (not saying that this is a complete example or already totally fine in any way):

class Placement {

public: // settings

    Vector3 position;
    Matrix3x3 orientation;

};

We could now build a Camera class and a Player class so that they have a Placement instance as members:

class Camera {

public: // 3D stuff

    Placement placement;

public: // camera specific stuff

    ...
};


class Player {

public: // 3D stuff

    Placement placement;

public: // player specific stuff

    ...

};

Now think of a class that is concerned with e.g. aiming (or parenting, or orbiting, or tracking, or ...).

class Aiming
    : public GeometricConstraint {

public:

    const Placement* pointOfInterest;
    Placement* target;

    virtual
    void update() override;

};

Notice that this class works with placements. It does not need to know whether a placement is part of a camera, a player object, or anything else. This is true for both the placement that is altered by the automatism (i.e. the target) as well as the placement that is used as auxiliary (i.e. the pointOfInterest). 

Oh wow,it does work now that i think about it...neat,Thanks.I’ll try it.