Fixed #471.

Bonuses from artifacts are now inherited. Some work on artifacts set GUI. CArtifactsOfHero now operates on its own hero copy when picking artifact. Still more fixes are needed though.

client/CHeroWindow.cpp

@@ -171,10 +171,6 @@ CHeroWindow::~CHeroWindow()
 
 	delete garr;
 	delete ourBar;
-
-	artifs->rollback();
-	delete artifs->commonInfo;
-	artifs->commonInfo = NULL; //to prevent heap corruption
 	delete artifs;
 
 	delete portraitArea;
@@ -532,7 +528,6 @@ void CHeroWindow::dispose()
 	curBack = NULL;
 	curHero = NULL;
 
-	artifs->rollback();
 	artifs->dispose();
 }
 

client/CPlayerInterface.cpp

@@ -945,10 +945,10 @@ void CPlayerInterface::openHeroWindow(const CGHeroInstance *hero)
 void CPlayerInterface::heroArtifactSetChanged(const CGHeroInstance*hero)
 {
 	boost::unique_lock<boost::recursive_mutex> un(*pim);
-	if(adventureInt->heroWindow->curHero) //hero window is opened
+	if(adventureInt->heroWindow->curHero && adventureInt->heroWindow->curHero->id == hero->id) //hero window is opened
 	{
 		adventureInt->heroWindow->deactivate();
-		adventureInt->heroWindow->setHero(adventureInt->heroWindow->curHero);
+		adventureInt->heroWindow->setHero(hero);
 		adventureInt->heroWindow->activate();
 		return;
 	}
@@ -957,8 +957,9 @@ void CPlayerInterface::heroArtifactSetChanged(const CGHeroInstance*hero)
 		cew->deactivate();
 		for(int g=0; g<ARRAY_COUNT(cew->heroInst); ++g)
 		{
-			if(cew->heroInst[g] == hero)
+			if(cew->heroInst[g]->id == hero->id)
 			{
+				cew->heroInst[g] = hero;
 				cew->artifs[g]->updateState = true;
 				cew->artifs[g]->setHero(hero);
 				cew->artifs[g]->updateState = false;

client/GUIClasses.cpp

@@ -4367,10 +4367,12 @@ void CArtPlace::clickLeft(tribool down, bool previousState)
 			GH.pushInt(spellWindow);
 		}
 	}
+
 	if (!down && previousState)
 	{
 		if(ourArt && ourArt->id == 0)
 			return; //this is handled separately
+
 		if(!ourOwner->commonInfo->srcAOH) //nothing has been clicked
 		{
 			if(ourArt) //to prevent selecting empty slots (bugfix to what GrayFace reported)
@@ -4431,10 +4433,11 @@ void CArtPlace::clickLeft(tribool down, bool previousState)
 				ourOwner->commonInfo->destArtifact = ourArt;
 
 				// Special case when the dest artifact can't be fit into the src slot.
-				//CGI->arth->unequipArtifact(ourOwner->curHero->artifWorn, slotID);
+				CGI->arth->unequipArtifact(ourOwner->curHero->artifWorn, slotID);
 				const CArtifactsOfHero* srcAOH = ourOwner->commonInfo->srcAOH;
 				ui16 srcSlotID = ourOwner->commonInfo->srcSlotID;
-				if (ourArt && srcSlotID < 19 && !ourArt->fitsAt(srcAOH->curHero->artifWorn, srcSlotID)) {
+				if (ourArt && srcSlotID < 19 && !ourArt->fitsAt(srcAOH->curHero->artifWorn, srcSlotID)) 
+				{
 					// Put dest artifact into owner's backpack.
 					ourOwner->commonInfo->srcAOH = ourOwner;
 					ourOwner->commonInfo->srcSlotID = ourOwner->curHero->artifacts.size() + 19;
@@ -4448,19 +4451,14 @@ void CArtPlace::clickLeft(tribool down, bool previousState)
 			}
 		}
 	}
-	/*else if(!down && clicked)
-	{
-		if(ourArt && ourArt->id == 0)
-			return; //this is handled separately
-		deselect();
-	}*/
-	//ClickableL::clickLeft(down);
 }
 
 void CArtPlace::clickRight(tribool down, bool previousState)
 {
-	if(down && ourArt && !locked() && text.size()) { //if there is no description or it's a lock, do nothing ;]
-		if (slotID < 19) {
+	if(down && ourArt && !locked() && text.size())  //if there is no description or it's a lock, do nothing ;]
+	{
+		if (slotID < 19) 
+		{
 			selectedNo = false;
 
 			// If the artifact can be assembled, display dialog.
@@ -4480,7 +4478,8 @@ void CArtPlace::clickRight(tribool down, bool previousState)
 			}
 
 			// Otherwise if the artifact can be diasassembled, display dialog.
-			if (ourArt->constituents != NULL) {
+			if (ourArt->constituents != NULL) 
+			{
 				LOCPLINT->showArtifactAssemblyDialog(
 					ourArt->id,
 					0,
@@ -4522,11 +4521,11 @@ void CArtPlace::select ()
 	ourOwner->commonInfo->srcAOH = ourOwner;
 
 	// Temporarily remove artifact from hero.
-	//if (slotID < 19)
-	//	CGI->arth->unequipArtifact(ourOwner->curHero->artifWorn, slotID);
-	//else
-	//	ourOwner->curHero->artifacts.erase(ourOwner->curHero->artifacts.begin() + (slotID - 19));
-	//ourOwner->markPossibleSlots(ourArt);
+	if (slotID < 19)
+		CGI->arth->unequipArtifact(ourOwner->curHero->artifWorn, slotID);
+	else
+		ourOwner->curHero->artifacts.erase(ourOwner->curHero->artifacts.begin() + (slotID - 19));
+	ourOwner->markPossibleSlots(ourArt);
 	//ourOwner->curHero->recreateArtBonuses();
 
 	// Update the hero bonuses.
@@ -4548,6 +4547,13 @@ void CArtPlace::select ()
 				cew->artifs[g]->setHero(ourOwner->curHero);
 			}
 		}
+
+		//use our copy of hero to draw window
+		if(cew->heroInst[0]->id == ourOwner->curHero->id)
+			cew->heroInst[0] = ourOwner->curHero;
+		else
+			cew->heroInst[1] = ourOwner->curHero;
+
 		cew->prepareBackground();
 		cew->activate();
 	}
@@ -4694,61 +4700,23 @@ void LRClickableAreaOpenTown::clickRight(tribool down, bool previousState)
 		LOCPLINT->openTownWindow(town);//TODO: popup?
 }
 
-void CArtifactsOfHero::activate()
-{
-	for(size_t f=0; f<artWorn.size(); ++f)
-	{
-		if(artWorn[f])
-			artWorn[f]->activate();
-	}
-	for(size_t f=0; f<backpack.size(); ++f)
-	{
-		if(backpack[f])
-			backpack[f]->activate();
-	}
-	
-	leftArtRoll->activate();
-	rightArtRoll->activate();
-}
-
-void CArtifactsOfHero::deactivate()
+void CArtifactsOfHero::SCommonPart::reset()
 {
-	for(size_t f=0; f<artWorn.size(); ++f)
-	{
-		if(artWorn[f])
-			artWorn[f]->deactivate();
-	}
-	for(size_t f=0; f<backpack.size(); ++f)
-	{
-		if(backpack[f])
-			backpack[f]->deactivate();
-	}
-	
-	leftArtRoll->deactivate();
-	rightArtRoll->deactivate();
-}
-
-void CArtifactsOfHero::show(SDL_Surface * to)
-{
-	for(size_t d=0; d<artWorn.size(); ++d)
-	{
-		artWorn[d]->show(to);
-	}
-	for(size_t d=0; d<backpack.size(); ++d)
-	{
-		backpack[d]->show(to);
-	}
-
-	leftArtRoll->show(to);
-	rightArtRoll->show(to);
+	destAOH = srcAOH = NULL;
+	destArtifact = srcArtifact = NULL;
+	destSlotID = srcSlotID = -1;
 }
 
 void CArtifactsOfHero::setHero(const CGHeroInstance * hero)
 {
 	// An update is made, rather than initialization.
-	if (curHero == hero) 
+	if (curHero && curHero->id == hero->id) 
 	{
-		curHero = hero;
+		if(curHero != hero)
+		{
+			delete	curHero;
+			curHero = new CGHeroInstance(*hero);
+		}
 
 		// Compensate backpack pos if an artifact was insertad before it.
 		if (commonInfo->destSlotID >= 19 && commonInfo->destAOH == this
@@ -4757,14 +4725,16 @@ void CArtifactsOfHero::setHero(const CGHeroInstance * hero)
 			backpackPos++;
 		}
 
-		if (updateState && commonInfo->srcAOH == this) {
+		if (updateState && commonInfo->srcAOH == this) 
+		{
 			// A swap was made, make the replaced artifact the current selected.
-			if (commonInfo->destSlotID < 19 && commonInfo->destArtifact) {
+			if (commonInfo->destSlotID < 19 && commonInfo->destArtifact) 
+			{
 				// Temporarily remove artifact from hero.
-				//if (commonInfo->srcSlotID < 19)
-				//	CGI->arth->unequipArtifact(curHero->artifWorn, commonInfo->srcSlotID);
-				//else
-				//	curHero->artifacts.erase(curHero->artifacts.begin() + (commonInfo->srcSlotID - 19));
+				if (commonInfo->srcSlotID < 19)
+					CGI->arth->unequipArtifact(curHero->artifWorn, commonInfo->srcSlotID);
+				else
+					curHero->artifacts.erase(curHero->artifacts.begin() + (commonInfo->srcSlotID - 19));
 				//curHero->recreateArtBonuses();
 
 				// Source <- Dest
@@ -4775,20 +4745,13 @@ void CArtifactsOfHero::setHero(const CGHeroInstance * hero)
 				commonInfo->destArtifact = NULL;
 				commonInfo->destSlotID = -1;
 
-				CGI->curh->dragAndDropCursor(
-					graphics->artDefs->ourImages[commonInfo->srcArtifact->id].bitmap);
+				CGI->curh->dragAndDropCursor(graphics->artDefs->ourImages[commonInfo->srcArtifact->id].bitmap);
 				markPossibleSlots(commonInfo->srcArtifact);
 			} 
 			else if (commonInfo->destAOH != NULL) 
 			{
 				// Reset all parameters.
-				commonInfo->srcAOH = NULL;
-				commonInfo->srcArtifact = NULL;
-				commonInfo->srcSlotID = -1;
-				commonInfo->destAOH = NULL;
-				commonInfo->destArtifact = NULL;
-				commonInfo->destSlotID = -1;
-
+				commonInfo->reset();
 				CGI->curh->dragAndDropCursor(NULL);
 				unmarkSlots();
 			}
@@ -4796,10 +4759,14 @@ void CArtifactsOfHero::setHero(const CGHeroInstance * hero)
 	} 
 	else 
 	{
-		rollback();
+		commonInfo->reset();
 	}
 
-	curHero = const_cast<CGHeroInstance *>(hero);
+	if(hero != curHero)
+	{
+		delete curHero;
+		curHero = new CGHeroInstance(*hero);
+	}
 
 	if (curHero->artifacts.size() > 0)
 		backpackPos %= curHero->artifacts.size();
@@ -4816,67 +4783,16 @@ void CArtifactsOfHero::setHero(const CGHeroInstance * hero)
 	rightArtRoll->block(curHero->artifacts.size() <= backpack.size());
 }
 
-/**
- * Any held artifacts, marked slots etc. will be restored to it's original way.
- */
-void CArtifactsOfHero::rollback()
-{
-	if (curHero != NULL) 
-	{
-		// Restore any held artifact to it's original position.
-		if (commonInfo->srcArtifact && commonInfo->srcAOH == this) 
-		{
-			if (commonInfo->srcSlotID != -1) {
-				// Put a held artifact back to it's spot.
-				//if (commonInfo->srcSlotID < 19)
-				//	CGI->arth->equipArtifact(curHero->artifWorn, commonInfo->srcSlotID, commonInfo->srcArtifact->id);
-				//else
-				//	curHero->artifacts.insert(curHero->artifacts.begin() + (commonInfo->srcSlotID - 19), commonInfo->srcArtifact->id);
-			} 
-			else  // Held swapped artifact.
-			{
-				// Wear the artifact in a suitable spot.
-				ui16 i = 0;
-				for (; i < 19; i++) 
-				{
-					if (artWorn[i]->fitsHere(commonInfo->srcArtifact) && !vstd::contains(curHero->artifWorn, i)) 
-					{
-						//CGI->arth->equipArtifact(curHero->artifWorn, i, commonInfo->srcArtifact->id);
-						break;
-					}
-				}
-
-				// If it can't be worn, put it in the backpack.
-				if (i == 19)
-					;//curHero->artifacts.push_back(commonInfo->srcArtifact->id);
-			}
-
-			//curHero->recreateArtBonuses();
-		}
-	}
-
-	unmarkSlots();
-	backpackPos = 0;
-
-	commonInfo->srcAOH = NULL;
-	commonInfo->srcArtifact = NULL;
-	commonInfo->srcSlotID = -1;
-	commonInfo->destAOH = NULL;
-	commonInfo->destArtifact = NULL;
-	commonInfo->destSlotID = -1;
-
-	CGI->curh->dragAndDropCursor(NULL);
-}
-
 void CArtifactsOfHero::dispose()
 {
-	curHero = NULL;
+	delNull(curHero);
 }
 
 void CArtifactsOfHero::scrollBackpack(int dir)
 {
 	backpackPos += dir;
-	if (curHero->artifacts.size() > 0) {
+	if (curHero->artifacts.size() > 0) 
+	{
 		if (backpackPos < 0) { // No guarantee of modulus behavior with negative operands.
 			do {
 				backpackPos += curHero->artifacts.size();
@@ -4887,7 +4803,8 @@ void CArtifactsOfHero::scrollBackpack(int dir)
 	}
 
 	//set new data
-	for (size_t s = 0; s < backpack.size(); ++s) {
+	for (size_t s = 0; s < backpack.size(); ++s) 
+	{
 		if (s < curHero->artifacts.size())
 			setSlotData(backpack[s], 19 + (s + backpackPos)%curHero->artifacts.size());
 		else
@@ -4906,7 +4823,8 @@ void CArtifactsOfHero::markPossibleSlots (const CArtifact* art)
 		it != commonInfo->participants.end();
 		++it)
 	{
-		for (int i = 0; i < (*it)->artWorn.size(); i++) {
+		for (int i = 0; i < (*it)->artWorn.size(); i++) 
+		{
 			if ((*it)->artWorn[i]->fitsHere(art))
 				(*it)->artWorn[i]->marked = true;
 			else
@@ -4924,7 +4842,8 @@ void CArtifactsOfHero::unmarkSlots ()
 		it != commonInfo->participants.end();
 		++it)
 	{
-		for (int i = 0; i < (*it)->artWorn.size(); i++) {
+		for (int i = 0; i < (*it)->artWorn.size(); i++) 
+		{
 			(*it)->artWorn[i]->marked = false;
 		}
 	}
@@ -4938,13 +4857,16 @@ void CArtifactsOfHero::setSlotData (CArtPlace* artPlace, int slotID)
 	artPlace->slotID = slotID;
 	artPlace->ourArt = curHero->getArt(slotID);
 
-	if (artPlace->ourArt) {
+	if (artPlace->ourArt) 
+	{
 		artPlace->text = artPlace->ourArt->Description();
 		if (artPlace->locked()) // Locks should appear as empty.
 			artPlace->hoverText = CGI->generaltexth->allTexts[507];
 		else
 			artPlace->hoverText = boost::str(boost::format(CGI->generaltexth->heroscrn[1].c_str()) % artPlace->ourArt->Name().c_str());
-	} else {
+	} 
+	else 
+	{
 		eraseSlotData(artPlace, slotID);
 	}
 }
@@ -4961,25 +4883,26 @@ void CArtifactsOfHero::eraseSlotData (CArtPlace* artPlace, int slotID)
 }
 
 CArtifactsOfHero::CArtifactsOfHero(const Point &position) :
-	backpackPos(0), updateState(false), commonInfo(NULL)
+	backpackPos(0), updateState(false), commonInfo(NULL), curHero(NULL)
 {
+	OBJ_CONSTRUCTION_CAPTURING_ALL;
 	pos += position;
 	artWorn.resize(19);
 	
-	std::vector<SDL_Rect> slotPos;
-	slotPos += genRect(44,44,pos.x+509,pos.y+30), genRect(44,44,pos.x+567,pos.y+240), genRect(44,44,pos.x+509,pos.y+80), 
-		genRect(44,44,pos.x+383,pos.y+68), genRect(44,44,pos.x+564,pos.y+183), genRect(44,44,pos.x+509,pos.y+130), 
-		genRect(44,44,pos.x+431,pos.y+68), genRect(44,44,pos.x+610,pos.y+183), genRect(44,44,pos.x+515,pos.y+295), 
-		genRect(44,44,pos.x+383,pos.y+143), genRect(44,44,pos.x+399,pos.y+194), genRect(44,44,pos.x+415,pos.y+245),
-		genRect(44,44,pos.x+431,pos.y+296), genRect(44,44,pos.x+564,pos.y+30), genRect(44,44,pos.x+610,pos.y+30), 
-		genRect(44,44,pos.x+610,pos.y+76), genRect(44,44,pos.x+610,pos.y+122), genRect(44,44,pos.x+610,pos.y+310),	
-		genRect(44,44,pos.x+381,pos.y+296);
+	std::vector<Rect> slotPos;
+	slotPos += genRect(44,44,509,30), genRect(44,44,567,240), genRect(44,44,509,80), 
+		genRect(44,44,383,68), genRect(44,44,564,183), genRect(44,44,509,130), 
+		genRect(44,44,431,68), genRect(44,44,610,183), genRect(44,44,515,295), 
+		genRect(44,44,383,143), genRect(44,44,399,194), genRect(44,44,415,245),
+		genRect(44,44,431,296), genRect(44,44,564,30), genRect(44,44,610,30), 
+		genRect(44,44,610,76), genRect(44,44,610,122), genRect(44,44,610,310),	
+		genRect(44,44,381,296);
 
 	// Create slots for worn artifacts.
 	for (int g = 0; g < 19 ; g++)
 	{	
 		artWorn[g] = new CArtPlace(NULL);
-		artWorn[g]->pos = slotPos[g];
+		artWorn[g]->pos = slotPos[g] + pos;
 		artWorn[g]->ourOwner = this;
 		eraseSlotData(artWorn[g], g);
 	}
@@ -4998,28 +4921,14 @@ CArtifactsOfHero::CArtifactsOfHero(const Point &position) :
 		backpack.push_back(add);
 	}
 
-	leftArtRoll = new AdventureMapButton(std::string(), std::string(), boost::bind(&CArtifactsOfHero::scrollBackpack,this,-1), pos.x+379, pos.y+364, "hsbtns3.def", SDLK_LEFT);
-	rightArtRoll = new AdventureMapButton(std::string(), std::string(), boost::bind(&CArtifactsOfHero::scrollBackpack,this,+1), pos.x+632, pos.y+364, "hsbtns5.def", SDLK_RIGHT);
+	leftArtRoll = new AdventureMapButton(std::string(), std::string(), boost::bind(&CArtifactsOfHero::scrollBackpack,this,-1), 379, 364, "hsbtns3.def", SDLK_LEFT);
+	rightArtRoll = new AdventureMapButton(std::string(), std::string(), boost::bind(&CArtifactsOfHero::scrollBackpack,this,+1), 632, 364, "hsbtns5.def", SDLK_RIGHT);
 }
 
 CArtifactsOfHero::~CArtifactsOfHero()
 {
 	dispose();
-	for(size_t g=0; g<artWorn.size(); ++g)
-	{
-		delete artWorn[g];
-		artWorn[g] = NULL;
-	}
-	for(size_t g=0; g<backpack.size(); ++g)
-	{
-		delete backpack[g];
-		backpack[g] = NULL;
-	}
-	backpack.clear();
-	artWorn.clear();
-
-	delete leftArtRoll;
-	delete rightArtRoll;
+	CGI->curh->dragAndDropCursor(NULL);
 }
 
 void CExchangeWindow::close()
@@ -5324,8 +5233,6 @@ CExchangeWindow::~CExchangeWindow() //d-tor
 	delete quit;
 
 	//warning: don't experiment with these =NULL lines, they prevent heap corruption!
-	artifs[0]->rollback();
-	artifs[1]->rollback();
 	delete artifs[0]->commonInfo;
 	artifs[0]->commonInfo = NULL;
 	delete artifs[0];

client/GUIClasses.h

@@ -909,7 +909,7 @@ inline bool CArtPlace::locked () const
 
 class CArtifactsOfHero : public CIntObject
 {
-	const CGHeroInstance * curHero;
+	CGHeroInstance * curHero; //local copy of hero on which we operate
 
 	std::vector<CArtPlace *> artWorn; // 0 - head; 1 - shoulders; 2 - neck; 3 - right hand; 4 - left hand; 5 - torso; 6 - right ring; 7 - left ring; 8 - feet; 9 - misc1; 10 - misc2; 11 - misc3; 12 - misc4; 13 - mach1; 14 - mach2; 15 - mach3; 16 - mach4; 17 - spellbook; 18 - misc5
 	std::vector<CArtPlace *> backpack; //hero's visible backpack (only 5 elements!)
@@ -925,19 +925,16 @@ public:
 		const CArtifactsOfHero * destAOH; // For swapping. (i.e. changing what is held)
 		int destSlotID;	                  // Needed to determine what kind of action was last taken in setHero
 		const CArtifact * destArtifact;   // For swapping.
+
+		void reset();
 	} * commonInfo; //when we have more than one CArtifactsOfHero in one window with exchange possibility, we use this (eg. in exchange window); to be provided externally
 
 	bool updateState; // Whether the commonInfo should be updated on setHero or not.
 
 	AdventureMapButton * leftArtRoll, * rightArtRoll;
 
-	void activate();
-	void deactivate();
-	void show(SDL_Surface * to);
-
 	void setHero(const CGHeroInstance * hero);
 	void dispose(); //free resources not needed after closing windows and reset state
-	void rollback();
 	void scrollBackpack(int dir); //dir==-1 => to left; dir==1 => to right
 	void markPossibleSlots (const CArtifact* art);
 	void unmarkSlots ();

client/NetPacksClient.cpp

@@ -326,17 +326,17 @@ void SetHeroArtifacts::applyCl( CClient *cl )
 	if(!player)
 		return;
 
-	h->recreateArtBonuses();
+	//h->recreateArtBonuses();
 	player->heroArtifactSetChanged(h);
 
-	BOOST_FOREACH(Bonus bonus, gained)
-	{
-		player->heroBonusChanged(h,bonus,true);
-	}
-	BOOST_FOREACH(Bonus bonus, lost)
-	{
-		player->heroBonusChanged(h,bonus,false);
-	}
+// 	BOOST_FOREACH(Bonus bonus, gained)
+// 	{
+// 		player->heroBonusChanged(h,bonus,true);
+// 	}
+// 	BOOST_FOREACH(Bonus bonus, lost)
+// 	{
+// 		player->heroBonusChanged(h,bonus,false);
+// 	}
 }
 
 void HeroRecruited::applyCl( CClient *cl )

hch/CArtHandler.cpp

@@ -87,7 +87,8 @@ bool CArtifact::fitsAt (const std::map<ui16, ui32> &artifWorn, ui16 slotID) cons
 		}
 
 		// Ensure enough ring slots are free
-		for (int i = 0; i < sizeof(ringSlots)/sizeof(*ringSlots); i++) {
+		for (int i = 0; i < sizeof(ringSlots)/sizeof(*ringSlots); i++) 
+		{
 			if (tempArtifWorn.find(ringSlots[i]) == tempArtifWorn.end() || ringSlots[i] == slotID)
 				rings--;
 		}
@@ -95,7 +96,8 @@ bool CArtifact::fitsAt (const std::map<ui16, ui32> &artifWorn, ui16 slotID) cons
 			return false;
 
 		// Ensure enough misc slots are free.
-		for (int i = 0; i < sizeof(miscSlots)/sizeof(*miscSlots); i++) {
+		for (int i = 0; i < sizeof(miscSlots)/sizeof(*miscSlots); i++) 
+		{
 			if (tempArtifWorn.find(miscSlots[i]) == tempArtifWorn.end() || miscSlots[i] == slotID)
 				misc--;
 		}
@@ -114,10 +116,13 @@ bool CArtifact::canBeAssembledTo (const std::map<ui16, ui32> &artifWorn, ui32 ar
 	const CArtifact &artifact = *VLC->arth->artifacts[artifactID];
 	assert(artifact.constituents);
 
-	BOOST_FOREACH(ui32 constituentID, *artifact.constituents) {
+	BOOST_FOREACH(ui32 constituentID, *artifact.constituents) 
+	{
 		bool found = false;
-		for (std::map<ui16, ui32>::const_iterator it = artifWorn.begin(); it != artifWorn.end(); ++it) {
-			if (it->second == constituentID) {
+		for (std::map<ui16, ui32>::const_iterator it = artifWorn.begin(); it != artifWorn.end(); ++it) 
+		{
+			if (it->second == constituentID) 
+			{
 				found = true;
 				break;
 			}
@@ -129,43 +134,13 @@ bool CArtifact::canBeAssembledTo (const std::map<ui16, ui32> &artifWorn, ui32 ar
 	return true;
 }
 
-/**
- * Adds all the bonuses of this artifact, including possible constituents, to
- * a bonus list.
- */
-void CArtifact::addBonusesTo (BonusList *otherBonuses) const
+CArtifact::CArtifact()
 {
-	for(std::list<Bonus>::const_iterator i = bonuses.begin(); i != bonuses.end(); i++)
-		otherBonuses->push_back(*i);
-
-	if (constituents != NULL) {
-		BOOST_FOREACH(ui32 artifactID, *constituents) 
-		{
-			VLC->arth->artifacts[artifactID]->addBonusesTo(otherBonuses);
-		}
-	}
+	nodeType = ARTIFACT;
 }
 
-/**
- * Removes all the bonuses of this artifact, including possible constituents, from
- * a bonus list.
- */
-void CArtifact::removeBonusesFrom (BonusList *otherBonuses) const
+CArtifact::~CArtifact()
 {
-	if (constituents != NULL) {
-		BOOST_FOREACH(ui32 artifactID, *constituents) {
-			VLC->arth->artifacts[artifactID]->removeBonusesFrom(otherBonuses);
-		}
-	}
-
-	while (1) {
-		std::list<Bonus>::iterator it = std::find_if(otherBonuses->begin(), otherBonuses->end(),boost::bind(Bonus::IsFrom,_1,Bonus::ARTIFACT,id));
-
-		if (it != otherBonuses->end())
-			otherBonuses->erase(it);
-		else
-			break;
-	}
 }
 
 CArtHandler::CArtHandler()
@@ -709,10 +684,9 @@ void CArtHandler::clear()
  * @param artifWorn A hero's set of worn artifacts.
  * @param bonuses Optional list of bonuses to update.
  */
-void CArtHandler::equipArtifact
-	(std::map<ui16, ui32> &artifWorn, ui16 slotID, ui32 artifactID, BonusList *bonuses)
+void CArtHandler::equipArtifact(std::map<ui16, ui32> &artifWorn, ui16 slotID, ui32 artifactID)
 {
-	unequipArtifact(artifWorn, slotID, bonuses);
+	unequipArtifact(artifWorn, slotID);
 
 	const CArtifact &artifact = *artifacts[artifactID];
 
@@ -745,9 +719,6 @@ void CArtHandler::equipArtifact
 			}
 		}
 	}
-
-	if (bonuses != NULL)
-		artifact.addBonusesTo(bonuses);
 }
 
 /**
@@ -756,8 +727,7 @@ void CArtHandler::equipArtifact
  * @param artifWorn A hero's set of worn artifacts.
  * @param bonuses Optional list of bonuses to update.
  */
-void CArtHandler::unequipArtifact
-	(std::map<ui16, ui32> &artifWorn, ui16 slotID, BonusList *bonuses)
+void CArtHandler::unequipArtifact(std::map<ui16, ui32> &artifWorn, ui16 slotID)
 {
 	if (!vstd::contains(artifWorn, slotID))
 		return;
@@ -793,7 +763,4 @@ void CArtHandler::unequipArtifact
 			}
 		}
 	}
-
-	if (bonuses != NULL)
-		artifact.removeBonusesFrom(bonuses);
 }

hch/CArtHandler.h

@@ -18,7 +18,7 @@
  */
 class CDefHandler;
 
-class DLL_EXPORT CArtifact //container for artifacts
+class DLL_EXPORT CArtifact : public CBonusSystemNode //container for artifacts
 {
 	std::string name, description; //set if custom
 public:
@@ -37,12 +37,15 @@ public:
 	std::vector<ui32> * constituentOf; // Reverse map of constituents.
 	EartClass aClass;
 	si32 id;
-	std::list<Bonus> bonuses; //bonuses given by artifact
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
-		h & name & description & price & possibleSlots & constituents & constituentOf & aClass & id & bonuses;
+		h & static_cast<CBonusSystemNode&>(*this);;
+		h & name & description & price & possibleSlots & constituents & constituentOf & aClass & id;
 	}
+
+	CArtifact();
+	~CArtifact();
 };
 
 class DLL_EXPORT CArtHandler //handles artifacts
@@ -64,8 +67,8 @@ public:
 	void getAllowed(std::vector<CArtifact*> &out, int flags);
 	void erasePickedArt (si32 id);
 	bool isBigArtifact (ui32 artID) {return bigArtifacts.find(artID) != bigArtifacts.end();}
-	void equipArtifact (std::map<ui16, ui32> &artifWorn, ui16 slotID, ui32 artifactID, BonusList *bonuses = NULL);
-	void unequipArtifact (std::map<ui16, ui32> &artifWorn, ui16 slotID, BonusList *bonuses = NULL);
+	void equipArtifact (std::map<ui16, ui32> &artifWorn, ui16 slotID, ui32 artifactID);
+	void unequipArtifact (std::map<ui16, ui32> &artifWorn, ui16 slotID);
 	static int convertMachineID(int id, bool creToArt);
 	CArtHandler();
 	~CArtHandler();

hch/CObjectHandler.cpp

@@ -762,9 +762,9 @@ void CGHeroInstance::initHero()
 
 	if(!vstd::contains(artifWorn, 16) && type->startingSpell >= 0) //no catapult means we haven't read pre-existant set
 	{
-		VLC->arth->equipArtifact(artifWorn, 17, 0, &bonuses); //give spellbook
+		VLC->arth->equipArtifact(artifWorn, 17, 0); //give spellbook
 	}
-	VLC->arth->equipArtifact(artifWorn, 16, 3, &bonuses); //everyone has a catapult
+	VLC->arth->equipArtifact(artifWorn, 16, 3); //everyone has a catapult
 
 	if(portrait < 0 || portrait == 255)
 		portrait = subID;
@@ -800,7 +800,6 @@ void CGHeroInstance::initHero()
 	boost::algorithm::replace_first(hoverName,"%s",name);
 	boost::algorithm::replace_first(hoverName,"%s", type->heroClass->name);
 
-	recreateArtBonuses();
 	if(mana < 0)
 		mana = manaLimit(); //after all bonuses are taken into account
 }
@@ -833,14 +832,13 @@ void CGHeroInstance::initArmy(CCreatureSet *dst /*= NULL*/)
 			switch (creID)
 			{
 			case 145: //catapult
-				VLC->arth->equipArtifact(artifWorn, 16, 3, &bonuses);
+				VLC->arth->equipArtifact(artifWorn, 16, 3);
 				break;
 			default:
 				VLC->arth->equipArtifact(
 					artifWorn,
 					9+CArtHandler::convertMachineID(creID,true),
-					CArtHandler::convertMachineID(creID,true),
-					&bonuses);
+					  CArtHandler::convertMachineID(creID,true));
 				break;
 			}
 		}
@@ -937,7 +935,7 @@ void CGHeroInstance::initObj()
 	if(!type)
 		return; //TODO support prison
 
-	for (std::vector<specialInfo>::iterator it = type->spec.begin(); it != type->spec.end(); it++)
+	for (std::vector<specialInfo>::const_iterator it = type->spec.begin(); it != type->spec.end(); it++)
 	{
 		bonus.val = it->val;
 		bonus.id = id; //from the hero, speciality has no unique id
@@ -1386,7 +1384,7 @@ void CGHeroInstance::giveArtifact (ui32 aid)
 		{
 			if (!vstd::contains(artifWorn, *it)) 
 			{
-				VLC->arth->equipArtifact(artifWorn, *it, aid, &bonuses);
+				VLC->arth->equipArtifact(artifWorn, *it, aid);
 				break;
 			}
 		}
@@ -1397,17 +1395,6 @@ void CGHeroInstance::giveArtifact (ui32 aid)
 	}
 }
 
-void CGHeroInstance::recreateArtBonuses()
-{
-	//clear all bonuses from artifacts (if present) and give them again
-	bonuses.remove_if(boost::bind(Bonus::IsFrom,_1,Bonus::ARTIFACT,0xffffff));
-	for (std::map<ui16,ui32>::iterator ari = artifWorn.begin(); ari != artifWorn.end(); ari++)
-	{
-		CArtifact &art = *VLC->arth->artifacts[ari->second];
-		art.addBonusesTo(&bonuses);
-	}
-}
-
 bool CGHeroInstance::hasArt( ui32 aid ) const
 {
 	if(vstd::contains(artifacts, aid))
@@ -1455,7 +1442,10 @@ void CGHeroInstance::getParents(TCNodes &out, const CBonusSystemNode *root /*= N
 	if((root == this || contains(static_cast<const CStackInstance *>(root))) &&  visitedTown)
 		out.insert(visitedTown);
 
-	out.insert (&speciality);
+	for (std::map<ui16,ui32>::const_iterator i = artifWorn.begin(); i != artifWorn.end(); i++)
+		out.insert(VLC->arth->artifacts[i->second]);
+
+	out.insert(&speciality);
 }
 
 void CGHeroInstance::pushPrimSkill(int which, int val)

hch/CObjectHandler.h

@@ -275,7 +275,7 @@ public:
 
 	//////////////////////////////////////////////////////////////////////////
 
-	CHero * type;
+	const CHero * type;
 	ui64 exp; //experience points
 	si32 level; //current level of hero
 	std::string name; //may be custom
@@ -286,8 +286,8 @@ public:
 	si32 movement; //remaining movement points
 	ui8 sex;
 	ui8 inTownGarrison; // if hero is in town garrison 
-	CGTownInstance * visitedTown; //set if hero is visiting town or in the town garrison
-	CGBoat *boat; //set to CGBoat when sailing
+	const CGTownInstance * visitedTown; //set if hero is visiting town or in the town garrison
+	const CGBoat *boat; //set to CGBoat when sailing
 	std::vector<ui32> artifacts; //hero's artifacts from bag
 	std::map<ui16,ui32> artifWorn; //map<position,artifact_id>; positions: 0 - head; 1 - shoulders; 2 - neck; 3 - right hand; 4 - left hand; 5 - torso; 6 - right ring; 7 - left ring; 8 - feet; 9 - misc1; 10 - misc2; 11 - misc3; 12 - misc4; 13 - mach1; 14 - mach2; 15 - mach3; 16 - mach4; 17 - spellbook; 18 - misc5
 	std::set<ui32> spells; //known spells (spell IDs)
@@ -376,7 +376,6 @@ public:
 	void initHero(); 
 	void initHero(int SUBID); 
 	void initArmy(CCreatureSet *dst = NULL);
-	void recreateArtBonuses();
 	void giveArtifact (ui32 aid);
 	void initHeroDefInfo();
 	void pushPrimSkill(int which, int val);

lib/CGameState.cpp

@@ -1484,10 +1484,8 @@ void CGameState::init( StartInfo * si, ui32 checksum, int Seed )
 
 				CGHeroInstance *hero = k->second.heroes[0];
 				std::vector<ui16>::iterator slot = vstd::findFirstNot (hero->artifWorn, toGive->possibleSlots);
-				if(slot!=toGive->possibleSlots.end())
-				{
-					VLC->arth->equipArtifact(hero->artifWorn, *slot, toGive->id, &hero->bonuses);
-				}
+				if(slot != toGive->possibleSlots.end())
+					VLC->arth->equipArtifact(hero->artifWorn, *slot, toGive->id);
 				else
  					hero->giveArtifact(toGive->id);
 			}

lib/HeroBonus.h

@@ -377,7 +377,7 @@ public:
 
 	enum ENodeTypes
 	{
-		UNKNOWN, STACK, SPECIALITY
+		UNKNOWN, STACK, SPECIALITY, ARTIFACT
 	};
 };
 

lib/NetPacks.h

@@ -588,6 +588,7 @@ struct SetHeroArtifacts : public CPackForClient //509
 		h & hid & artifacts & artifWorn;
 	}
 
+	std::vector<ui32> equiped, unequiped; //used locally
 	BonusList gained, lost; //used locally as hlp when applying
 };   
 

lib/NetPacksLib.cpp

@@ -301,12 +301,12 @@ DLL_EXPORT void RemoveObject::applyGs( CGameState *gs )
 		gs->getPlayer(player)->heroes.erase(nitr);
 		h->tempOwner = 255; //no one owns beaten hero
 
-		if(h->visitedTown)
+		if(CGTownInstance *t = const_cast<CGTownInstance *>(h->visitedTown))
 		{
 			if(h->inTownGarrison)
-				h->visitedTown->garrisonHero = NULL;
+				t->garrisonHero = NULL;
 			else
-				h->visitedTown->visitingHero = NULL;
+				t->visitingHero = NULL;
 			h->visitedTown = NULL;
 		}
 
@@ -382,10 +382,11 @@ void TryMoveHero::applyGs( CGameState *gs )
 	}
 	else if(result == DISEMBARK) //hero leaves boat to dest tile
 	{
-		h->boat->direction = h->moveDir;
-		h->boat->pos = start;
-		h->boat->hero = NULL;
-		gs->map->addBlockVisTiles(h->boat);
+		CGBoat *b = const_cast<CGBoat *>(h->boat);
+		b->direction = h->moveDir;
+		b->pos = start;
+		b->hero = NULL;
+		gs->map->addBlockVisTiles(b);
 		h->boat = NULL;
 	}
 
@@ -393,8 +394,8 @@ void TryMoveHero::applyGs( CGameState *gs )
 	{
 		gs->map->removeBlockVisTiles(h);
 		h->pos = end;
-		if(h->boat)
-			h->boat->pos = end;
+		if(CGBoat *b = const_cast<CGBoat *>(h->boat))
+			b->pos = end;
 		gs->map->addBlockVisTiles(h);
 	}
 
@@ -413,8 +414,9 @@ DLL_EXPORT void SetGarrisons::applyGs( CGameState *gs )
 		else if(ai->ID==HEROI_TYPE)
 		{
 			CGHeroInstance *h =  static_cast<CGHeroInstance*>(ai);
-			if(h->visitedTown && h->inTownGarrison)
-				h->visitedTown->setArmy(i->second);
+			CGTownInstance *t = const_cast<CGTownInstance *>(h->visitedTown);
+			if(t && h->inTownGarrison)
+				t->setArmy(i->second);
 		}
 	}
 }
@@ -470,7 +472,6 @@ DLL_EXPORT void SetHeroesInTown::applyGs( CGameState *gs )
 DLL_EXPORT void SetHeroArtifacts::applyGs( CGameState *gs )
 {
 	CGHeroInstance *h = gs->getHero(hid);
-	std::vector<ui32> equiped, unequiped;
 	for(std::map<ui16,ui32>::const_iterator i = h->artifWorn.begin(); i != h->artifWorn.end(); i++)
 		if(!vstd::contains(artifWorn,i->first)  ||  artifWorn[i->first] != i->second)
 			unequiped.push_back(i->second);
@@ -479,37 +480,14 @@ DLL_EXPORT void SetHeroArtifacts::applyGs( CGameState *gs )
 		if(!vstd::contains(h->artifWorn,i->first)  ||  h->artifWorn[i->first] != i->second)
 			equiped.push_back(i->second);
 
-	BOOST_FOREACH(ui32 id, equiped)
-	{
-		//if hero already had equipped at least one artifact of that type, don't give any new bonuses
-		if(h->getArtPos(id) >= 0)
-			continue;
-
-		CArtifact &art = *VLC->arth->artifacts[id];
-		art.addBonusesTo(&h->bonuses);
-		art.addBonusesTo(&gained);
-	}
-
 	//update hero data
 	h->artifacts = artifacts;
 	h->artifWorn = artifWorn;
-
-	//remove bonus from unequipped artifact
-	BOOST_FOREACH(ui32 id, unequiped)
-	{
-		//if hero still has equipped at least one artifact of that type, don't remove bonuses
-		if(h->getArtPos(id) >= 0)
-			continue;
-
-		CArtifact &art = *VLC->arth->artifacts[id];
-		art.removeBonusesFrom(&h->bonuses);
-		art.addBonusesTo(&lost);
-	}
 }
 
 DLL_EXPORT void SetHeroArtifacts::setArtAtPos(ui16 pos, int art)
 {
-	if(art<0)
+	if(art < 0)
 	{
 		if(pos<19)
 			VLC->arth->unequipArtifact(artifWorn, pos);
@@ -518,9 +496,12 @@ DLL_EXPORT void SetHeroArtifacts::setArtAtPos(ui16 pos, int art)
 	}
 	else
 	{
-		if (pos < 19) {
+		if (pos < 19) 
+		{
 			VLC->arth->equipArtifact(artifWorn, pos, (ui32) art);
-		} else { // Goes into the backpack.
+		} 
+		else // Goes into the backpack.
+		{ 
 			if(pos - 19 < artifacts.size())
 				artifacts.insert(artifacts.begin() + (pos - 19), art);
 			else

lib/map.cpp

@@ -920,39 +920,39 @@ void Mapa::loadHero( CGObjectInstance * &nobj, const unsigned char * bufor, int
 		for(int pom=0;pom<16;pom++)
 		{
 			int id = readNormalNr(bufor,i, artidlen); i+=artidlen;
-			if(id!=artmask)
-				VLC->arth->equipArtifact(nhi->artifWorn, pom, id, &nhi->bonuses);
+			if(id != artmask)
+				VLC->arth->equipArtifact(nhi->artifWorn, pom, id);
 		}
 		//misc5 art //17
 		if(version>=SoD)
 		{
 			int id = readNormalNr(bufor,i, artidlen); i+=artidlen;
 			if(id!=artmask)
-				VLC->arth->equipArtifact(nhi->artifWorn, 16, id, &nhi->bonuses);
+				VLC->arth->equipArtifact(nhi->artifWorn, 16, id);
 			else
-				VLC->arth->equipArtifact(nhi->artifWorn, 16, 3, &nhi->bonuses); //catapult by default
+				VLC->arth->equipArtifact(nhi->artifWorn, 16, 3); //catapult by default
 		}
 		//spellbook
 		int id = readNormalNr(bufor,i, artidlen); i+=artidlen;
 		if(id!=artmask)
-			VLC->arth->equipArtifact(nhi->artifWorn, 17, id, &nhi->bonuses);
+			VLC->arth->equipArtifact(nhi->artifWorn, 17, id);
 		//19 //???what is that? gap in file or what? - it's probably fifth slot..
 		if(version>RoE)
 		{
 			id = readNormalNr(bufor,i, artidlen); i+=artidlen;
 			if(id!=artmask)
-				VLC->arth->equipArtifact(nhi->artifWorn, 18, id, &nhi->bonuses);
+				VLC->arth->equipArtifact(nhi->artifWorn, 18, id);
 		}
 		else
 			i+=1;
 		//bag artifacts //20
 		int amount = readNormalNr(bufor,i, 2); i+=2; //number of artifacts in hero's bag
-		if(amount>0)
+		if(amount > 0)
 		{
-			for(int ss=0; ss<amount; ++ss)
+			for(int ss = 0; ss < amount; ++ss)
 			{
 				id = readNormalNr(bufor,i, artidlen); i+=artidlen;
-				if(id!=artmask)
+				if(id != artmask)
 					nhi->giveArtifact(id);
 			}
 		}
@@ -1160,7 +1160,7 @@ void Mapa::readPredefinedHeroes( const unsigned char * bufor, int &i)
 					{
 						int id = readNormalNr(bufor,i, artidlen); i+=artidlen;
 						if(id!=artmask)
-							VLC->arth->equipArtifact(cgh->artifWorn, pom, id, &cgh->bonuses);
+							VLC->arth->equipArtifact(cgh->artifWorn, pom, id);
 					}
 					//misc5 art //17
 					if(version>=SoD)
@@ -1173,13 +1173,13 @@ void Mapa::readPredefinedHeroes( const unsigned char * bufor, int &i)
 					//spellbook
 					int id = readNormalNr(bufor,i, artidlen); i+=artidlen;
 					if(id!=artmask)
-						VLC->arth->equipArtifact(cgh->artifWorn, 17, id, &cgh->bonuses);
+						VLC->arth->equipArtifact(cgh->artifWorn, 17, id);
 					//19 //???what is that? gap in file or what? - it's probably fifth slot..
 					if(version>RoE)
 					{
 						id = readNormalNr(bufor,i, artidlen); i+=artidlen;
 						if(id!=artmask)
-							VLC->arth->equipArtifact(cgh->artifWorn, 18, id, &cgh->bonuses);
+							VLC->arth->equipArtifact(cgh->artifWorn, 18, id);
 					}
 					else
 						i+=1;

server/CGameHandler.cpp

@@ -2926,7 +2926,8 @@ bool CGameHandler::swapArtifacts(si32 srcHeroID, si32 destHeroID, ui16 srcSlot,
 	const CArtifact *srcArtifact = srcHero->getArt(srcSlot);
 	const CArtifact *destArtifact = destHero->getArt(destSlot);
 
-	if (srcArtifact == NULL) {
+	if (srcArtifact == NULL)
+	{
 		complain("No artifact to swap!");
 		return false;
 	}
@@ -2937,7 +2938,8 @@ bool CGameHandler::swapArtifacts(si32 srcHeroID, si32 destHeroID, ui16 srcSlot,
 	sha.artifWorn = srcHero->artifWorn;
 
 	// Combinational artifacts needs to be removed first so they don't get denied movement because of their own locks.
-	if (srcHeroID == destHeroID && srcSlot < 19 && destSlot < 19) {
+	if (srcHeroID == destHeroID && srcSlot < 19 && destSlot < 19) 
+	{
 		sha.setArtAtPos(srcSlot, -1);
 		if (!vstd::contains(sha.artifWorn, destSlot))
 			destArtifact = NULL;
@@ -2952,17 +2954,20 @@ bool CGameHandler::swapArtifacts(si32 srcHeroID, si32 destHeroID, ui16 srcSlot,
 		return false;
 	}
 
-	if ((srcArtifact && srcArtifact->id == 145) || (destArtifact && destArtifact->id == 145)) {
+	if ((srcArtifact && srcArtifact->id == 145) || (destArtifact && destArtifact->id == 145)) 
+	{
 		complain("Cannot move artifact locks.");
 		return false;
 	}
 
-	if (destSlot >= 19 && srcArtifact->isBig()) {
+	if (destSlot >= 19 && srcArtifact->isBig()) 
+	{
 		complain("Cannot put big artifacts in backpack!");
 		return false;
 	}
 
-	if (srcSlot == 16 || destSlot == 16) {
+	if (srcSlot == 16 || destSlot == 16) 
+	{
 		complain("Cannot move catapult!");
 		return false;
 	}
@@ -2979,7 +2984,8 @@ bool CGameHandler::swapArtifacts(si32 srcHeroID, si32 destHeroID, ui16 srcSlot,
 		sha.setArtAtPos(srcSlot, destArtifact ? destArtifact->id : -1);
 
 	// Internal hero artifact arrangement.
-	if(srcHero == destHero) {
+	if(srcHero == destHero) 
+	{
 		// Correction for destination from removing source artifact in backpack.
 		if (srcSlot >= 19 && destSlot >= 19 && srcSlot < destSlot)
 			destSlot--;
@@ -2987,7 +2993,8 @@ bool CGameHandler::swapArtifacts(si32 srcHeroID, si32 destHeroID, ui16 srcSlot,
 		sha.setArtAtPos(destSlot, srcHero->getArtAtPos(srcSlot));
 	}
 	sendAndApply(&sha);
-	if (srcHeroID != destHeroID) {
+	if (srcHeroID != destHeroID) 
+	{
 		// Exchange between two different heroes.
 		sha.hid = destHeroID;
 		sha.artifacts = destHero->artifacts;
@@ -3112,7 +3119,7 @@ bool CGameHandler::assembleArtifacts (si32 heroID, ui16 artifactSlot, bool assem
 bool CGameHandler::buyArtifact( ui32 hid, si32 aid )
 {
 	CGHeroInstance *hero = gs->getHero(hid);
-	CGTownInstance *town = hero->visitedTown;
+	CGTownInstance *town = const_cast<CGTownInstance*>(hero->visitedTown);
 	if(aid==0) //spellbook
 	{
 		if(!vstd::contains(town->builtBuildings,si32(0)) && complain("Cannot buy a spellbook, no mage guild in the town!")