Merge pull request #202 from vcmi/noncopyableBonusSystemNode2

Didn't manage to find any crashes in newly created games. Merging.

Keep in mind that even if most of old saves will load properly some of them still corrupted beyond repair and after some turns they might eventually crash.

lib/CGameState.cpp

@@ -1080,13 +1080,14 @@ void CGameState::initPlayerStates()
 	logGlobal->debug("\tCreating player entries in gs");
 	for(auto & elem : scenarioOps->playerInfos)
 	{
-		std::pair<PlayerColor, PlayerState> ins(elem.first,PlayerState());
-		ins.second.color=ins.first;
-		ins.second.human = elem.second.playerID;
-		ins.second.team = map->players[ins.first.getNum()].team;
-		teams[ins.second.team].id = ins.second.team;//init team
-		teams[ins.second.team].players.insert(ins.first);//add player to team
-		players.insert(ins);
+		PlayerState & p = players[elem.first];
+		//std::pair<PlayerColor, PlayerState> ins(elem.first,PlayerState());
+		p.color=elem.first;
+		p.human = elem.second.playerID;
+		p.team = map->players[elem.first.getNum()].team;
+		teams[p.team].id = p.team;//init team
+		teams[p.team].players.insert(elem.first);//add player to team
+		//players.insert(ins);
 	}
 }
 
@@ -2936,6 +2937,25 @@ PlayerState::PlayerState()
 	setNodeType(PLAYER);
 }
 
+PlayerState::PlayerState(PlayerState && other):
+	CBonusSystemNode(std::move(other)),
+	color(other.color),
+	human(other.human),
+	team(other.team),
+	resources(other.resources),
+	enteredWinningCheatCode(other.enteredWinningCheatCode),
+	enteredLosingCheatCode(other.enteredLosingCheatCode),
+	status(other.status),
+	daysWithoutCastle(other.daysWithoutCastle)
+{
+	std::swap(visitedObjects, other.visitedObjects);
+	std::swap(heroes, other.heroes);
+	std::swap(towns, other.towns);
+	std::swap(availableHeroes, other.availableHeroes);
+	std::swap(dwellings, other.dwellings);
+	std::swap(quests, other.quests);
+}
+
 std::string PlayerState::nodeName() const
 {
 	return "Player " + (color.getNum() < VLC->generaltexth->capColors.size() ? VLC->generaltexth->capColors[color.getNum()] : boost::lexical_cast<std::string>(color));
@@ -3235,6 +3255,15 @@ TeamState::TeamState()
 	setNodeType(TEAM);
 }
 
+TeamState::TeamState(TeamState && other):
+	CBonusSystemNode(std::move(other)),
+	id(other.id)
+{
+	std::swap(players, other.players);
+	std::swap(fogOfWarMap, other.fogOfWarMap);
+}
+
+
 CRandomGenerator & CGameState::getRandomGenerator()
 {
 	//logGlobal->traceStream() << "Fetching CGameState::rand with seed " << rand.nextInt();

lib/CPlayerState.h

@@ -35,6 +35,8 @@ public:
 	boost::optional<ui8> daysWithoutCastle;
 
 	PlayerState();
+	PlayerState(PlayerState && other);
+
 	std::string nodeName() const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
@@ -56,6 +58,7 @@ public:
 	std::vector<std::vector<std::vector<ui8> > >  fogOfWarMap; //true - visible, false - hidden
 
 	TeamState();
+	TeamState(TeamState && other);
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{

lib/HeroBonus.cpp

@@ -96,6 +96,13 @@ BonusList::BonusList(const BonusList &bonusList)
 	belongsToTree = false;
 }
 
+BonusList::BonusList(BonusList&& other):
+	belongsToTree(false)
+{
+	std::swap(belongsToTree, other.belongsToTree);
+	std::swap(bonuses, other.bonuses);
+}
+
 BonusList& BonusList::operator=(const BonusList &bonusList)
 {
 	bonuses.resize(bonusList.size());
@@ -104,6 +111,12 @@ BonusList& BonusList::operator=(const BonusList &bonusList)
 	return *this;
 }
 
+void BonusList::changed()
+{
+    if(belongsToTree)
+		CBonusSystemNode::treeHasChanged();
+}
+
 int BonusList::totalValue() const
 {
 	int base = 0;
@@ -257,24 +270,19 @@ void BonusList::eliminateDuplicates()
 void BonusList::push_back(Bonus* const &x)
 {
 	bonuses.push_back(x);
-
-	if (belongsToTree)
-		CBonusSystemNode::treeHasChanged();
+	changed();
 }
 
 std::vector<Bonus*>::iterator BonusList::erase(const int position)
 {
-	if (belongsToTree)
-		CBonusSystemNode::treeHasChanged();
+	changed();
 	return bonuses.erase(bonuses.begin() + position);
 }
 
 void BonusList::clear()
 {
 	bonuses.clear();
-
-	if (belongsToTree)
-		CBonusSystemNode::treeHasChanged();
+	changed();
 }
 
 std::vector<BonusList*>::size_type BonusList::operator-=(Bonus* const &i)
@@ -283,26 +291,20 @@ std::vector<BonusList*>::size_type BonusList::operator-=(Bonus* const &i)
 	if(itr == bonuses.end())
 		return false;
 	bonuses.erase(itr);
-
-	if (belongsToTree)
-		CBonusSystemNode::treeHasChanged();
+	changed();
 	return true;
 }
 
 void BonusList::resize(std::vector<Bonus*>::size_type sz, Bonus* c )
 {
 	bonuses.resize(sz, c);
-
-	if (belongsToTree)
-		CBonusSystemNode::treeHasChanged();
+	changed();
 }
 
 void BonusList::insert(std::vector<Bonus*>::iterator position, std::vector<Bonus*>::size_type n, Bonus* const &x)
 {
 	bonuses.insert(position, n, x);
-
-	if (belongsToTree)
-		CBonusSystemNode::treeHasChanged();
+	changed();
 }
 
 int IBonusBearer::valOfBonuses(Bonus::BonusType type, const CSelector &selector) const
@@ -708,6 +710,36 @@ CBonusSystemNode::CBonusSystemNode() : bonuses(true), exportedBonuses(true), nod
 {
 }
 
+CBonusSystemNode::CBonusSystemNode(CBonusSystemNode && other):
+	bonuses(std::move(other.bonuses)),
+	exportedBonuses(std::move(other.exportedBonuses)),
+	nodeType(other.nodeType),
+	description(other.description),
+	cachedLast(0)
+{
+	std::swap(parents, other.parents);
+	std::swap(children, other.children);
+
+	//fixing bonus tree without recalculation
+
+	for(CBonusSystemNode * n : parents)
+	{
+		n->children -= &other;
+		n->children.push_back(this);
+	}
+
+	for(CBonusSystemNode * n : children)
+	{
+		n->parents -= &other;
+		n->parents.push_back(this);
+	}
+
+	//cache ignored
+
+	//cachedBonuses
+	//cachedRequests
+}
+
 CBonusSystemNode::~CBonusSystemNode()
 {
 	detachFromAll();

lib/HeroBonus.h

@@ -412,7 +412,7 @@ private:
 
 	TInternalContainer bonuses;
 	bool belongsToTree;
-
+	void changed();
 
 public:
 	typedef TInternalContainer::const_reference const_reference;
@@ -423,6 +423,7 @@ public:
 
 	BonusList(bool BelongsToTree = false);
 	BonusList(const BonusList &bonusList);
+	BonusList(BonusList && other);
 	BonusList& operator=(const BonusList &bonusList);
 
 	// wrapper functions of the STL vector container
@@ -494,12 +495,8 @@ public:
 	{
 		return bonuses.end();
 	}
-
-	//friend inline std::vector<Bonus*>::iterator range_begin(BonusList & x);
-	//friend inline std::vector<Bonus*>::iterator range_end(BonusList & x);
 };
 
-
 // Extensions for BOOST_FOREACH to enable iterating of BonusList objects
 // Don't touch/call this functions
 inline BonusList::iterator range_begin(BonusList & x)
@@ -614,7 +611,7 @@ public:
 	const TBonusListPtr getSpellBonuses() const;
 };
 
-class DLL_LINKAGE CBonusSystemNode : public IBonusBearer
+class DLL_LINKAGE CBonusSystemNode : public IBonusBearer, public boost::noncopyable
 {
 public:
 	enum ENodeTypes
@@ -647,8 +644,8 @@ private:
 	const TBonusListPtr getAllBonusesWithoutCaching(const CSelector &selector, const CSelector &limit, const CBonusSystemNode *root = nullptr) const;
 
 public:
-
 	explicit CBonusSystemNode();
+	CBonusSystemNode(CBonusSystemNode && other);
 	virtual ~CBonusSystemNode();
 
 	void limitBonuses(const BonusList &allBonuses, BonusList &out) const; //out will bo populed with bonuses that are not limited here
@@ -1010,23 +1007,5 @@ template <class InputIterator>
 void BonusList::insert(const int position, InputIterator first, InputIterator last)
 {
 	bonuses.insert(bonuses.begin() + position, first, last);
-
-	if (belongsToTree)
-		CBonusSystemNode::treeHasChanged();
+	changed();
 }
-
-// Extensions for BOOST_FOREACH to enable iterating of BonusList objects
-/*namespace boost
-{
-	template<>
-	struct range_mutable_iterator<BonusList>
-	{
-		typedef std::vector<Bonus*>::iterator type;
-	};
-
-	template<>
-	struct range_const_iterator<BonusList>
-	{
-		typedef std::vector<Bonus*>::const_iterator type;
-	};
-}*/