Hello,

I'm creating a text based RPG game in C++. I'm trying to move the player creation code in main to a function below it. I've created a pointer to a character object and calling a function to return the reference of the newly created character. Is there a problem with doing it this way?

I ask because I'm having issues elsewhere in code relating to the persistence of the player's inventory. I add items to the inventory vector using a function for example, but they don't stay added.

Also, I've tried not using a function and passing references around to create the player character and everything works just fine, so that's what makes me suspicious of the code below....

Thanks!

Player* createNewChar();
int main(){

...

Player* pPlayer = 0;
...

}
...
Player* createNewChar(){

  string pName = selectName();

  cout << "Welcome, " << pName << ".\n";

  string pClass = selectClass();

  cout << "Creating your new character...!";

  Player player(pName, pClass);

  return &player;
}

...

Player* createNewChar(){
  ...
  Player player(pName, pClass);
  return &player;
}

This will fail indeed. It constructs the 'player' object, compute its pointer, walk out the function, destroy all contents of variables inside 'createNewChar' (since you just left it, C++ cleans it up for you), and return the just computed pointer (which now points to garbage) to the function that performed the call.

You can either use 'new' to create a pointer in the heap that won't get destroyed when you exit the function, or you can just give the player object to its caller.

BTW: Why is the function called "createCharacter" if you create a player?

As an example, in gcc or clang, I use the following flags.

-Wall (to include all warnings)

-Werror (to error if there is a warning)

-pedantic (to notify me if I am using non-standard C or C++).

Towards the end of a project, I also include:

-Wextra (aditional warning flags for things like (-Wclobbered, -Wempty-body))

These flags are explained further in the manpage

https://linux.die.net/man/1/gcc

Player* createNewChar(){
  ...
  Player player(pName, pClass);
  return &player;
}

This will fail indeed. It constructs the 'player' object, compute its pointer, walk out the function, destroy all contents of variables inside 'createNewChar' (since you just left it, C++ cleans it up for you), and return the just computed pointer (which now points to garbage) to the function that performed the call.

You can either use 'new' to create a pointer in the heap that won't get destroyed when you exit the function, or you can just give the player object to its caller.

BTW: Why is the function called "createCharacter" if you create a player?

This, basically.

Instead of declaring a pointer an assigning it via a function return, use the new keyword instead.

Player *player = new Player();

Then, in the Player constructor, add in all the additional code that should be processed, such as asking for a name, defining the class and whatnot. At least that's what I'm assuming your createCharacter function was going to handle. Just wrap that into the Player constructor.

edit: Also make sure you delete and remove the pointers when you no longer need them. I usually add a delete everytime I add a new to help mitigate wild pointers. If you declare any new pointers in a constructor, just remember to delete them in the destructor.

The solution is not "use new", or you'll shoot yourself in the foot in twenty even more painful ways.

You should learn how to use new and delete, and then promptly learn to never use it except in rare situations that, as a beginner, you're unqualified to recognize.

After learning about new and delete - but not using them (except in tiny pieces of practice code) - then look into std::unique_ptr / std::make_unique and std::shared_ptr / std::make_shared and use those instead.

Those template classes wrap proper usage of new and delete for you, and are much safer - even for experienced programmers.

The solution is not "use new", or you'll shoot yourself in the foot in twenty even more painful ways.

You should learn how to use new and delete, and then promptly learn to never use it except in rare situations that, as a beginner, you're unqualified to recognize.

After learning about new and delete - but not using them (except in tiny pieces of practice code) - then look into std::unique_ptr / std::make_unique and std::shared_ptr / std::make_shared and use those instead.

Those template classes wrap proper usage of new and delete for you, and are much safer - even for experienced programmers.

pointer, especially heap allocated ones, are only necessary for very large objects (like textures or meshes). your player class is just a collection of strings and integers. you could get away fine with just copying the values around. but if you wanted to know the most proper way to do this:

class player
{
  player(string name, string class)//a constructor that ask for that info up front
 {
   pName = name;
  pClass = class;
 }

 player() // empty constructor calls a prompting function
  {
    pName = promptforName();
   pClass = promptforClass;
  }

 //other fuctions and variables
};


int main()
{
  //if your person class was very big then youd want a pointer to it;
  player * myCharacter(); // a stack alloccated pointer to a player class, will be deleted when it goes out of scope
 //if you need a heap allocated fuction follow Velocity Raptors advice and use a shared_ptr like this

 auto pSmartPointer = make_shared<player>(myCharacter);// this function turns a regular pointer into a smart one.
// also deleted automatically. requires #include<memory> 
//correct way but not recommended
player * myCharacter_ptr = new player(name,class);

delete myCharacter_ptr; // you must delete this one manually.
avoid using new unless you need to.
return 0;
}