Merge pull request #1 from vcmi/develop

Update from original

client/CCastleInterface.cpp

@@ -1358,13 +1358,9 @@ std::string CBuildWindow::getTextForState(int state)
 			{
 				return town->town->buildings.at(build)->Name();
 			};
-			/*auto toBool = [&](const BuildingID build)
-			{
-				return town->hasBuilt(build);
-			};*/
 
 			ret = CGI->generaltexth->allTexts[52];
-			ret += "\n" + building->requirements.toString(toStr);
+			ret += "\n" + town->genBuildingRequirements(building->bid).toString(toStr);
 			break;
 		}
 	case EBuildingState::MISSING_BASE:

client/Graphics.cpp

@@ -60,7 +60,7 @@ void Graphics::loadPaletteAndColors()
 		col.r = pals[startPoint++];
 		col.g = pals[startPoint++];
 		col.b = pals[startPoint++];
-		CSDL_Ext::colorSetAlpha(col,SDL_ALPHA_OPAQUE);	
+		CSDL_Ext::colorSetAlpha(col,SDL_ALPHA_OPAQUE);
 		startPoint++;
 		playerColorPalette[i] = col;
 	}
@@ -75,7 +75,8 @@ void Graphics::loadPaletteAndColors()
 		neutralColorPalette[i].r = reader.readUInt8();
 		neutralColorPalette[i].g = reader.readUInt8();
 		neutralColorPalette[i].b = reader.readUInt8();
-		CSDL_Ext::colorSetAlpha(neutralColorPalette[i], !reader.readUInt8());
+		reader.readUInt8(); // this is "flags" entry, not alpha
+		CSDL_Ext::colorSetAlpha(neutralColorPalette[i], SDL_ALPHA_OPAQUE);
 	}
 	//colors initialization
 	SDL_Color colors[]  = { 

client/gui/Fonts.cpp

@@ -164,7 +164,7 @@ void CBitmapFont::renderCharacter(SDL_Surface * surface, const BitmapChar & char
 			switch(srcLine[dx])
 			{
 			case 1: //black "shadow"
-				std::fill(dstPixel, dstPixel + bpp, 0);
+				colorPutter(dstPixel, 0, 0, 0);
 				break;
 			case 255: //text colour
 				colorPutter(dstPixel, color.r, color.g, color.b);

config/objects/creatureBanks.json

@@ -110,7 +110,7 @@
 			},
 			"dwarvenTreasury" : {
 				"index" : 1,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Dwarven Treasury",
 				"rmg" : {
 					"value"		: 2000,
@@ -197,7 +197,7 @@
 			},
 			"griffinConservatory" : {
 				"index" : 2,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Griffin Conservatory",
 				"rmg" : {
 					"value"		: 2000,
@@ -268,7 +268,7 @@
 			},
 			"inpCache" :  {
 				"index" : 3,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Imp Cache",
 				"rmg" : {
 					"value"		: 5000,
@@ -354,7 +354,7 @@
 			},
 			"medusaStore" : {
 				"index" : 4,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Medusa Stores",
 				"rmg" : {
 					"value"		: 1500,
@@ -441,7 +441,7 @@
 			},
 			"nagaBank" : {
 				"index" : 5,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Naga Bank",
 				"rmg" : {
 					"value"		: 3000,
@@ -528,7 +528,7 @@
 			},
 			"dragonflyHive" : {
 				"index" : 6,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Dragon Fly Hive",
 				"rmg" : {
 					"value"		: 9000,
@@ -605,7 +605,7 @@
 		"types" : {
 			"shipwreck" : {
 				"index" : 0,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Shipwreck",
 				"rmg" : {
 					"value"		: 2000,
@@ -696,7 +696,7 @@
 		"types" : {
 			"derelictShip" : {
 				"index" : 0,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Derelict Ship",
 				"rmg" : {
 					"value"		: 4000,
@@ -788,7 +788,7 @@
 		"types" : {
 			"crypt" : {
 				"index" : 0,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Crypt",
 				"rmg" : {
 					"value"		: 1000,
@@ -877,7 +877,7 @@
 		"types" : {
 			"dragonUtopia" : {
 				"index" : 0,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Dragon Utopia",
 				"rmg" : {
 					"value"		: 10000,
@@ -986,7 +986,7 @@
 		"types" : {
 			"pyramid" : {
 				"index" : 0,
-				"resetDuraition" : 28,
+				"resetDuration" : 28,
 				"name" : "Pyramid",
 				"rmg" : {
 					"value"		: 5000,
@@ -1011,4 +1011,4 @@
 			}
 		}
 	}
-}
+}

lib/CBattleCallback.cpp

@@ -1675,8 +1675,11 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleStackIsImmune(co
 		break;
 	}
 
-	if (spell->isRisingSpell())
+    if (spell->isRisingSpell() && spell->id != SpellID::SACRIFICE)
 	{
+        // following does apply to resurrect and animate dead(?) only
+        // for sacrifice health calculation and health limit check don't matter
+
 		if(subject->count >= subject->baseAmount)
 			return ESpellCastProblem::STACK_IMMUNE_TO_SPELL;
 		
@@ -1783,12 +1786,27 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleCanCastThisSpell
 			const CGHeroInstance * caster = battleGetFightingHero(side);
 			const CSpell::TargetInfo ti = spell->getTargetInfo(caster->getSpellSchoolLevel(spell));
 			bool targetExists = false;
-			for(const CStack * stack : battleGetAllStacks()) //dead stacks will be immune anyway
+            bool targetToSacrificeExists = false; // for sacrifice we have to check for 2 targets (one dead to resurrect and one living to destroy)
+
+            for(const CStack * stack : battleGetAllStacks()) //dead stacks will be immune anyway
 			{
 				bool immune =  ESpellCastProblem::OK != battleStackIsImmune(caster, spell, mode, stack);
 				bool casterStack = stack->owner == caster->getOwner();
 				
-				if(!immune)
+                if(spell->id == SpellID::SACRIFICE)
+                {
+                    if(!immune && casterStack)
+                    {
+                        if(stack->alive())
+                            targetToSacrificeExists = true;
+                        else
+                            targetExists = true;
+                        if(targetExists && targetToSacrificeExists)
+                            break;
+                    }
+                }
+                else if(!immune)
+                {
 					switch (spell->positiveness)
 					{
 					case CSpell::POSITIVE:
@@ -1810,8 +1828,9 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleCanCastThisSpell
 						}
 						break;
 					}
+                }
 			}
-			if(!targetExists)
+            if(!targetExists || (spell->id == SpellID::SACRIFICE && !targetExists && !targetToSacrificeExists))
 			{
 				return ESpellCastProblem::NO_APPROPRIATE_TARGET;
 			}

lib/CGameInfoCallback.cpp

@@ -403,20 +403,17 @@ EBuildingState::EBuildingState CGameInfoCallback::canBuildStructure( const CGTow
 			return EBuildingState::NO_WATER; //lack of water
 	}
 
-	auto buildTest = [&](const BuildingID & id)
+	auto buildTest = [&](BuildingID id) -> bool
 	{
 		return t->hasBuilt(id);
 	};
 
-	if (!building->requirements.test(buildTest))
+	if (!t->genBuildingRequirements(ID).test(buildTest))
 		return EBuildingState::PREREQUIRES;
 
 	if(t->builded >= VLC->modh->settings.MAX_BUILDING_PER_TURN)
 		return EBuildingState::CANT_BUILD_TODAY; //building limit
 
-	if (building->upgrade != BuildingID::NONE && !t->hasBuilt(building->upgrade))
-		return EBuildingState::MISSING_BASE;
-
 	//checking resources
 	if(!building->resources.canBeAfforded(getPlayer(t->tempOwner)->resources))
 		return EBuildingState::NO_RESOURCES; //lack of res

lib/LogicalExpression.h

@@ -45,6 +45,11 @@ namespace LogicalExpressionDetail
 
 			std::vector<Variant> expressions;
 
+			bool operator == (const Element & other) const
+			{
+				return expressions == other.expressions;
+			}
+
 			template <typename Handler>
 			void serialize(Handler & h, const int version)
 			{
@@ -147,39 +152,73 @@ namespace LogicalExpressionDetail
 
 	/// Simple foreach visitor
 	template <typename ContainedClass>
-	class ForEachVisitor : public boost::static_visitor<void>
+	class ForEachVisitor : public boost::static_visitor<typename ExpressionBase<ContainedClass>::Variant>
 	{
 		typedef ExpressionBase<ContainedClass> Base;
 
-		std::function<void(typename Base::Value &)> visitor;
+		std::function<typename Base::Variant(const typename Base::Value &)> visitor;
 
 	public:
-		ForEachVisitor(std::function<void(typename Base::Value &)> visitor):
+		ForEachVisitor(std::function<typename Base::Variant(const typename Base::Value &)> visitor):
 			visitor(visitor)
 		{}
 
-		//FIXME: duplicated code
-		void operator()(typename Base::OperatorAny & element) const
+		typename Base::Variant operator()(const typename Base::Value & element) const
 		{
-			for (auto & entry : element.expressions)
-				boost::apply_visitor(*this, entry);
+			return visitor(element);
 		}
 
-		void operator()(typename Base::OperatorAll & element) const
+		template <typename Type>
+		typename Base::Variant operator()(Type element) const
 		{
 			for (auto & entry : element.expressions)
-				boost::apply_visitor(*this, entry);
+				entry = boost::apply_visitor(*this, entry);
+			return element;
 		}
+	};
 
-		void operator()(typename Base::OperatorNone & element) const
+	/// Minimizing visitor that removes all redundant elements from variant (e.g. AllOf inside another AllOf can be merged safely)
+	template <typename ContainedClass>
+	class MinimizingVisitor : public boost::static_visitor<typename ExpressionBase<ContainedClass>::Variant>
+	{
+		typedef ExpressionBase<ContainedClass> Base;
+
+	public:
+		typename Base::Variant operator()(const typename Base::Value & element) const
 		{
-			for (auto & entry : element.expressions)
-				boost::apply_visitor(*this, entry);
+			return element;
 		}
 
-		void operator()(typename Base::Value & element) const
+		template <typename Type>
+		typename Base::Variant operator()(const Type & element) const
 		{
-			visitor(element);
+			Type ret;
+
+			for (auto & entryRO : element.expressions)
+			{
+				auto entry = boost::apply_visitor(*this, entryRO);
+
+				try
+				{
+					// copy entries from child of this type
+					auto sublist = boost::get<Type>(entry).expressions;
+					std::move(sublist.begin(), sublist.end(), std::back_inserter(ret.expressions));
+				}
+				catch (boost::bad_get &)
+				{
+					// different type (e.g. allOf vs oneOf) just copy
+					ret.expressions.push_back(entry);
+				}
+			}
+
+			for ( auto it = ret.expressions.begin(); it != ret.expressions.end();)
+			{
+				if (std::find(ret.expressions.begin(), it, *it) != it)
+					it = ret.expressions.erase(it); // erase duplicate
+				else
+					it++; // goto next
+			}
+			return ret;
 		}
 	};
 
@@ -370,16 +409,23 @@ public:
 		std::swap(data, expr.data);
 	}
 
-	Variant get()
+	Variant get() const
 	{
 		return data;
 	}
 
 	/// Simple visitor that visits all entries in expression
-	void forEach(std::function<void(Value &)> visitor)
+	Variant morph(std::function<Variant(const Value &)> morpher) const
+	{
+		LogicalExpressionDetail::ForEachVisitor<Value> visitor(morpher);
+		return boost::apply_visitor(visitor, data);
+	}
+
+	/// Minimizes expression, removing any redundant elements
+	void minimize()
 	{
-		LogicalExpressionDetail::ForEachVisitor<Value> testVisitor(visitor);
-		boost::apply_visitor(testVisitor, data);
+		LogicalExpressionDetail::MinimizingVisitor<Value> visitor;
+		data = boost::apply_visitor(visitor, data);
 	}
 
 	/// calculates if expression evaluates to "true".

lib/NetPacksLib.cpp

@@ -396,7 +396,7 @@ DLL_LINKAGE void RemoveObject::applyGs( CGameState *gs )
 
 	for (TriggeredEvent & event : gs->map->triggeredEvents)
 	{
-		auto patcher = [&](EventCondition & cond)
+		auto patcher = [&](EventCondition cond) -> EventExpression::Variant
 		{
 			if (cond.object == obj)
 			{
@@ -411,8 +411,9 @@ DLL_LINKAGE void RemoveObject::applyGs( CGameState *gs )
 					cond.value = 0; // destroyed object, from now on can not be fulfilled
 				}
 			}
+			return cond;
 		};
-		event.trigger.forEach(patcher);
+		event.trigger = event.trigger.morph(patcher);
 	}
 
 	gs->map->objects[id.getNum()].dellNull();

lib/mapObjects/CGTownInstance.cpp

@@ -1004,6 +1004,40 @@ bool CGTownInstance::hasBuilt(BuildingID buildingID) const
 	return vstd::contains(builtBuildings, buildingID);
 }
 
+CBuilding::TRequired CGTownInstance::genBuildingRequirements(BuildingID buildID) const
+{
+	const CBuilding * building = town->buildings.at(buildID);
+
+	std::function<CBuilding::TRequired::Variant(const BuildingID &)> dependTest =
+	[&](const BuildingID & id) -> CBuilding::TRequired::Variant
+	{
+		const CBuilding * build = town->buildings.at(id);
+
+		if (!hasBuilt(id))
+			return id;
+
+		if (build->upgrade != BuildingID::NONE && !hasBuilt(build->upgrade))
+			return build->upgrade;
+
+		return build->requirements.morph(dependTest);
+	};
+
+	CBuilding::TRequired::OperatorAll requirements;
+	if (building->upgrade != BuildingID::NONE)
+	{
+		const CBuilding * upgr = town->buildings.at(building->upgrade);
+
+		requirements.expressions.push_back(upgr->bid);
+		requirements.expressions.push_back(upgr->requirements.morph(dependTest));
+	}
+	requirements.expressions.push_back(building->requirements.morph(dependTest));
+
+	CBuilding::TRequired::Variant variant(requirements);
+	CBuilding::TRequired ret(variant);
+	ret.minimize();
+	return ret;
+}
+
 void CGTownInstance::addHeroToStructureVisitors( const CGHeroInstance *h, si32 structureInstanceID ) const
 {
 	if(visitingHero == h)

lib/mapObjects/CGTownInstance.h

@@ -232,6 +232,8 @@ public:
 	bool armedGarrison() const; //true if town has creatures in garrison or garrisoned hero
 	int getTownLevel() const;
 
+	CBuilding::TRequired genBuildingRequirements(BuildingID build) const;
+
 	void removeCapitols (PlayerColor owner) const;
 	void addHeroToStructureVisitors(const CGHeroInstance *h, si32 structureInstanceID) const; //hero must be visiting or garrisoned in town
 

lib/mapObjects/CObjectHandler.cpp

@@ -191,7 +191,10 @@ void CGObjectInstance::setType(si32 ID, si32 subID)
 	//recalculate blockvis tiles - new appearance might have different blockmap than before
 	cb->gameState()->map->removeBlockVisTiles(this, true);
 	auto handler = VLC->objtypeh->getHandlerFor(ID, subID);
-	appearance = handler->getTemplates(tile.terType).at(0);
+	if (!handler->getTemplates(tile.terType).empty())
+		appearance = handler->getTemplates(tile.terType)[0];
+	else
+		appearance = handler->getTemplates()[0]; // get at least some appearance since alternative is crash
 	cb->gameState()->map->addBlockVisTiles(this);
 }
 

lib/mapObjects/CRewardableObject.cpp

@@ -421,7 +421,7 @@ void CRewardableObject::setPropertyDer(ui8 what, ui32 val)
 
 void CRewardableObject::newTurn() const
 {
-	if (resetDuration != 0 && cb->getDate(Date::DAY) % resetDuration == 0)
+	if (resetDuration != 0 && cb->getDate(Date::DAY) % (resetDuration+1) == 0)
 		cb->setObjProperty(id, ObjProperty::REWARD_RESET, 0);
 }
 

lib/mapObjects/CommonConstructors.cpp

@@ -303,6 +303,7 @@ void CBankInstanceConstructor::configureObject(CGObjectInstance * object, CRando
 CBankInfo::CBankInfo(JsonVector config):
 	config(config)
 {
+	assert(!config.empty());
 }
 
 static void addStackToArmy(IObjectInfo::CArmyStructure & army, const CCreature * crea, si32 amount)

lib/mapObjects/CommonConstructors.h

@@ -184,4 +184,10 @@ public:
 	void configureObject(CGObjectInstance * object, CRandomGenerator & rng) const;
 
 	std::unique_ptr<IObjectInfo> getObjectInfo(ObjectTemplate tmpl) const;
+
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{
+		h & levels & bankResetDuration;
+		h & static_cast<CDefaultObjectTypeHandler<CBank>&>(*this);
+	}
 };

lib/mapObjects/JsonRandom.cpp

@@ -202,6 +202,7 @@ namespace JsonRandom
 				for (auto creaID : crea->upgrades)
 					info.allowedCreatures.push_back(VLC->creh->creatures[creaID]);
 			}
+			ret.push_back(info);
 		}
 		return ret;
 	}

lib/mapping/CMap.cpp

@@ -441,7 +441,7 @@ void CMap::checkForObjectives()
 	// NOTE: probably should be moved to MapFormatH3M.cpp
 	for (TriggeredEvent & event : triggeredEvents)
 	{
-		auto patcher = [&](EventCondition & cond)
+		auto patcher = [&](EventCondition cond) -> EventExpression::Variant
 		{
 			switch (cond.condition)
 			{
@@ -491,8 +491,9 @@ void CMap::checkForObjectives()
 				//break; case EventCondition::DAYS_WITHOUT_TOWN:
 				//break; case EventCondition::STANDARD_WIN:
 			}
+			return cond;
 		};
-		event.trigger.forEach(patcher);
+		event.trigger = event.trigger.morph(patcher);
 	}
 }
 

lib/mapping/MapFormatH3M.cpp

@@ -676,16 +676,17 @@ void CMapLoaderH3M::readAllowedArtifacts()
 	// Messy, but needed
 	for (TriggeredEvent & event : map->triggeredEvents)
 	{
-		auto patcher = [&](EventCondition & cond)
+		auto patcher = [&](EventCondition cond) -> EventExpression::Variant
 		{
 			if (cond.condition == EventCondition::HAVE_ARTIFACT ||
 				cond.condition == EventCondition::TRANSPORT)
 			{
 				map->allowedArtifact[cond.objectType] = false;
 			}
+			return cond;
 		};
 
-		event.trigger.forEach(patcher);
+		event.trigger = event.trigger.morph(patcher);
 	}
 }