I am building a game and I need to store in a dynamic array a player, every time that a player is created. I built a small piece of code to try it and I get a segmentation fault
when I try to delete
the table to insert the third player. I just can't realize why that happens: the header
file is:
#include <iostream>
#include <string>
#include <vector>
using namespace std;
class Player
{
private:
string PlayerName;
int PlayerScore;
public:
Player();
Player(string name, int s);
~Player() {};
string getPlayerName() {return PlayerName;}
int getPlayerScore() {return PlayerScore;}
void setPlayerName(string name){PlayerName = name;}
void setPlayerScore(int score){PlayerScore = score;}
};
class Game
{
private:
Player NewPlayer;
int NPlayers;
Player* PlayerList;
//vector <Player> PlayerList2;
public:
Game();
~Game();
void setNewPlayer(string, int);
void resizePlayerList();
void PrintList();
};
The class
file:
#include <iostream>
#include <string>
#include <cstring>
#include <memory>
#include "I.h"
using namespace std;
Player::Player()
{
PlayerName = "";
PlayerScore = 0;
}
Player::Player(string name, int s)
{
PlayerName = name;
PlayerScore = s;
}
Game::Game()
{
NPlayers = 0;
PlayerList = NULL;
};
Game::~Game() {};
void Game::setNewPlayer(string str, int scr)
{
NewPlayer = Player(str, scr);
resizePlayerList();
PrintList();
}
void Game::resizePlayerList() {
if(NewPlayer.getPlayerName() != "No Name")
{
int newSize = NPlayers 1;
Player* newArr = NULL;
newArr = new Player[newSize];
memcpy( newArr, PlayerList, NPlayers * sizeof(Player) );
NPlayers = newSize;
delete [] PlayerList;
PlayerList = newArr;
PlayerList[NPlayers-1] = NewPlayer;
}
}
void Game::PrintList()
{
Player player;
//cout << NPlayers << endl;
for(int i= 0; i < NPlayers; i )
{
player = PlayerList[i];
cout << player.getPlayerName() << " " << player.getPlayerScore() << endl;
}
}
The main
:
#include <iostream>
#include <string>
#include "I.h"
using namespace std;
int main()
{
Game NewGame;
NewGame.setNewPlayer("Peter",20);
NewGame.setNewPlayer("Someone Someone",30);
NewGame.setNewPlayer("Someone else",40);
return 0;
}
CodePudding user response:
The problem stems from here:
memcpy(newArr, PlayerList, NPlayers * sizeof(Player));
You cannot copy classes in this manner, unless they are Trivially Copyable (originally said POD, but as Yksisarvinen points out in the comments, memcpy is not quite that restrictive). You can fix this by using a loop to copy over the data instead:
for (int i = 0; i < NPlayers; i) {
newArr[i] = std::move(PlayerList[i]);
}
The better option though, is to use a std::vector<Player>
, then the entire resize function can be simplified to:
void Game::resizePlayerList() {
if (NewPlayer.getPlayerName() != "No Name") {
PlayerList.push_back(std::move(NewPlayer));
}
}