Refactoring of H3M loader to make HotA format support easier

- extracted low-level reader from MapFormatH3M class
- added separate structure to define version-specific values
- cleared up some H3M format edge cases
- replaced witch hut skill vector with set
- converted several fields to enum type

AI/Nullkiller/Analyzers/BuildAnalyzer.cpp

@@ -294,7 +294,7 @@ bool BuildAnalyzer::hasAnyBuilding(int32_t alignment, BuildingID bid) const
 {
 	for(auto tdi : developmentInfos)
 	{
-		if(tdi.town->alignment == alignment && tdi.town->hasBuilt(bid))
+		if(tdi.town->subID == alignment && tdi.town->hasBuilt(bid))
 			return true;
 	}
 

AI/Nullkiller/Engine/PriorityEvaluator.cpp

@@ -848,7 +848,7 @@ public:
 
 uint64_t RewardEvaluator::getUpgradeArmyReward(const CGTownInstance * town, const BuildingInfo & bi) const
 {
-	if(ai->buildAnalyzer->hasAnyBuilding(town->alignment, bi.id))
+	if(ai->buildAnalyzer->hasAnyBuilding(town->subID, bi.id))
 		return 0;
 
 	auto creaturesToUpgrade = ai->armyManager->getTotalCreaturesAvailable(bi.baseCreatureID);

client/CMT.cpp

@@ -117,6 +117,9 @@ void init()
 
 	logGlobal->info("Initializing VCMI_Lib: %d ms", tmh.getDiff());
 
+	// Debug code to load all maps on start
+	//ClientCommandManager commandController;
+	//commandController.processCommand("convert txt", false);
 }
 
 static void prog_version()

client/battle/BattleInterfaceClasses.cpp

@@ -327,7 +327,7 @@ BattleHero::BattleHero(const BattleInterface & owner, const CGHeroInstance * her
 	if(!hero->type->battleImage.empty())
 		animationPath = hero->type->battleImage;
 	else
-	if(hero->sex)
+	if(hero->gender == EHeroGender::FEMALE)
 		animationPath = hero->type->heroClass->imageBattleFemale;
 	else
 		animationPath = hero->type->heroClass->imageBattleMale;

client/lobby/SelectionTab.cpp

@@ -333,7 +333,7 @@ void SelectionTab::filter(int size, bool selectFirst)
 	{
 		for(auto elem : allItems)
 		{
-			if(elem->mapHeader && elem->mapHeader->version && (!size || elem->mapHeader->width == size))
+			if(elem->mapHeader && (!size || elem->mapHeader->width == size))
 				curItems.push_back(elem);
 		}
 	}
@@ -537,9 +537,11 @@ void SelectionTab::parseMaps(const std::unordered_set<ResourceID> & files)
 			auto mapInfo = std::make_shared<CMapInfo>();
 			mapInfo->mapInit(file.getName());
 
-			// ignore unsupported map versions (e.g. WoG maps without WoG)
+			EMapFormat maxSupported = static_cast<EMapFormat>(CGI->settings()->getInteger(EGameSettings::TEXTS_MAP_VERSION));
-			// but accept VCMI maps
+
-			if((mapInfo->mapHeader->version >= EMapFormat::VCMI) || (mapInfo->mapHeader->version <= CGI->settings()->getInteger(EGameSettings::TEXTS_MAP_VERSION)))
+			if(mapInfo->mapHeader->version == EMapFormat::VCMI || mapInfo->mapHeader->version <= maxSupported)
+				allItems.push_back(mapInfo);
+
 			allItems.push_back(mapInfo);
 		}
 		catch(std::exception & e)

client/windows/CHeroWindow.cpp

@@ -335,7 +335,7 @@ void CHeroWindow::update(const CGHeroInstance * hero, bool redrawNeeded)
 
 	formations->resetCallback();
 	//setting formations
-	formations->setSelected(curHero->formation);
+	formations->setSelected(curHero->formation == EArmyFormation::TIGHT ? 1 : 0);
 	formations->addCallback([=](int value){ LOCPLINT->cb->setFormation(curHero, value);});
 
 	morale->set(&heroWArt);

cmake_modules/VCMI_lib.cmake

@@ -77,7 +77,9 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/mapping/CMapOperation.cpp
 		${MAIN_LIB_DIR}/mapping/CMapService.cpp
 		${MAIN_LIB_DIR}/mapping/MapEditUtils.cpp
+		${MAIN_LIB_DIR}/mapping/MapFeaturesH3M.cpp
 		${MAIN_LIB_DIR}/mapping/MapFormatH3M.cpp
+		${MAIN_LIB_DIR}/mapping/MapReaderH3M.cpp
 		${MAIN_LIB_DIR}/mapping/MapFormatJson.cpp
 
 		${MAIN_LIB_DIR}/registerTypes/RegisterTypes.cpp
@@ -347,7 +349,9 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/mapping/CMapOperation.h
 		${MAIN_LIB_DIR}/mapping/CMapService.h
 		${MAIN_LIB_DIR}/mapping/MapEditUtils.h
+		${MAIN_LIB_DIR}/mapping/MapFeaturesH3M.h
 		${MAIN_LIB_DIR}/mapping/MapFormatH3M.h
+		${MAIN_LIB_DIR}/mapping/MapReaderH3M.h
 		${MAIN_LIB_DIR}/mapping/MapFormatJson.h
 
 		${MAIN_LIB_DIR}/registerTypes/RegisterTypes.h

lib/CCreatureSet.cpp

@@ -418,7 +418,10 @@ int CCreatureSet::stacksCount() const
 
 void CCreatureSet::setFormation(bool tight)
 {
-	formation = tight;
+	if (tight)
+		formation = EArmyFormation::TIGHT;
+	else
+		formation = EArmyFormation::WIDE;
 }
 
 void CCreatureSet::setStackCount(const SlotID & slot, TQuantity count)
@@ -694,7 +697,6 @@ void CStackInstance::init()
 	experience = 0;
 	count = 0;
 	type = nullptr;
-	idRand = -1;
 	_armyObj = nullptr;
 	setNodeType(STACK_INSTANCE);
 }
@@ -815,8 +817,7 @@ std::string CStackInstance::getQuantityTXT(bool capitalized) const
 
 bool CStackInstance::valid(bool allowUnrandomized) const
 {
-	bool isRand = (idRand != -1);
+	if(!randomStack)
-	if(!isRand)
 	{
 		return (type  &&  type == VLC->creh->objects[type->getId()]);
 	}
@@ -830,8 +831,6 @@ std::string CStackInstance::nodeName() const
 	oss << "Stack of " << count << " of ";
 	if(type)
 		oss << type->getNamePluralTextID();
-	else if(idRand >= 0)
-		oss << "[no type, idRand=" << idRand << "]";
 	else
 		oss << "[UNDEFINED TYPE]";
 
@@ -888,14 +887,13 @@ void CStackInstance::serializeJson(JsonSerializeFormat & handler)
 
 	if(handler.saving)
 	{
-		if(idRand > -1)
+		if(randomStack)
 		{
-			int level = idRand / 2;
+			int level = randomStack->level;
-
+			int upgrade = randomStack->upgrade;
-			boost::logic::tribool upgraded = (idRand % 2) > 0;
 
 			handler.serializeInt("level", level, 0);
-			handler.serializeBool("upgraded", upgraded);
+			handler.serializeInt("upgraded", upgrade, 0);
 		}
 	}
 	else
@@ -903,13 +901,13 @@ void CStackInstance::serializeJson(JsonSerializeFormat & handler)
 		//type set by CStackBasicDescriptor::serializeJson
 		if(type == nullptr)
 		{
-			int level = 0;
+			uint8_t level = 0;
-			bool upgraded = false;
+			uint8_t upgrade = 0;
 
 			handler.serializeInt("level", level, 0);
-			handler.serializeBool("upgraded", upgraded);
+			handler.serializeInt("upgrade", upgrade, 0);
 
-			idRand = level * 2 + static_cast<int>(upgraded);
+			randomStack = RandomStackInfo{ level, upgrade };
 		}
 	}
 }
@@ -946,7 +944,6 @@ void CCommanderInstance::init()
 	level = 1;
 	count = 1;
 	type = nullptr;
-	idRand = -1;
 	_armyObj = nullptr;
 	setNodeType (CBonusSystemNode::COMMANDER);
 	secondarySkills.resize (ECommander::SPELL_POWER + 1);

lib/CCreatureSet.h

@@ -69,11 +69,13 @@ protected:
 	const CArmedInstance *_armyObj; //stack must be part of some army, army must be part of some object
 
 public:
-	// hlp variable used during loading map, when object (hero or town) have creatures that must have same alignment.
+	struct RandomStackInfo
-	// idRand < 0 -> normal, non-random creature
+	{
-	// idRand / 2 -> level
+		uint8_t level;
-	// idRand % 2 -> upgrade number
+		uint8_t upgrade;
-	int idRand;
+	};
+	// helper variable used during loading map, when object (hero or town) have creatures that must have same alignment.
+	boost::optional<RandomStackInfo> randomStack;
 
 	const CArmedInstance * const & armyObj; //stack must be part of some army, army must be part of some object
 	TExpType experience;//commander needs same amount of exp as hero
@@ -200,13 +202,21 @@ public:
 	}
 };
 
+enum class EArmyFormation : uint8_t
+{
+	WIDE,
+	TIGHT
+};
+
 class DLL_LINKAGE CCreatureSet : public IArmyDescriptor //seven combined creatures
 {
 	CCreatureSet(const CCreatureSet &) = delete;
 	CCreatureSet &operator=(const CCreatureSet&);
 public:
+
+
 	TSlots stacks; //slots[slot_id]->> pair(creature_id,creature_quantity)
-	ui8 formation = 0; //0 - wide, 1 - tight
+	EArmyFormation formation = EArmyFormation::WIDE; //0 - wide, 1 - tight
 
 	CCreatureSet() = default; //Should be here to avoid compile errors
 	virtual ~CCreatureSet();

lib/CGameState.cpp

@@ -496,7 +496,7 @@ std::pair<Obj,int> CGameState::pickObject (CGObjectInstance *obj)
 		return std::make_pair(Obj::RESOURCE,getRandomGenerator().nextInt(6)); //now it's OH3 style, use %8 for mithril
 	case Obj::RANDOM_TOWN:
 		{
-			PlayerColor align = PlayerColor((dynamic_cast<CGTownInstance *>(obj))->alignment);
+			PlayerColor align = (dynamic_cast<CGTownInstance *>(obj))->alignmentToPlayer;
 			si32 f; // can be negative (for random)
 			if(align >= PlayerColor::PLAYER_LIMIT) //same as owner / random
 			{

lib/CHeroHandler.cpp

@@ -420,7 +420,7 @@ CHero * CHeroHandler::loadFromJson(const std::string & scope, const JsonNode & n
 	hero->ID = HeroTypeID(index);
 	hero->identifier = identifier;
 	hero->modScope = scope;
-	hero->sex = node["female"].Bool();
+	hero->gender = node["female"].Bool() ? EHeroGender::FEMALE : EHeroGender::MALE;
 	hero->special = node["special"].Bool();
 
 	VLC->generaltexth->registerString(scope, hero->getNameTextID(), node["texts"]["name"].String());

lib/CHeroHandler.h

@@ -29,6 +29,13 @@ class CRandomGenerator;
 class JsonSerializeFormat;
 class BattleField;
 
+enum class EHeroGender : uint8_t
+{
+	MALE = 0,
+	FEMALE = 1,
+	DEFAULT = 0xff // from h3m, instance has same gender as hero type
+};
+
 class DLL_LINKAGE CHero : public HeroType
 {
 	friend class CHeroHandler;
@@ -61,7 +68,7 @@ public:
 	std::set<SpellID> spells;
 	bool haveSpellBook = false;
 	bool special = false; // hero is special and won't be placed in game (unless preset on map), e.g. campaign heroes
-	ui8 sex = 0; // default sex: 0=male, 1=female
+	EHeroGender gender = EHeroGender::MALE; // default sex: 0=male, 1=female
 
 	/// Graphics
 	std::string iconSpecSmall;
@@ -104,7 +111,7 @@ public:
 		h & specialty;
 		h & spells;
 		h & haveSpellBook;
-		h & sex;
+		h & gender;
 		h & special;
 		h & iconSpecSmall;
 		h & iconSpecLarge;

lib/GameConstants.h

@@ -1079,6 +1079,7 @@ public:
 		FIRST_AID_TENT = 6,
 		//CENTAUR_AXE = 7,
 		//BLACKSHARD_OF_THE_DEAD_KNIGHT = 8,
+		VIAL_OF_DRAGON_BLOOD = 127,
 		ARMAGEDDONS_BLADE = 128,
 		TITANS_THUNDER = 135,
 		//CORNUCOPIA = 140,

lib/battle/BattleInfo.cpp

@@ -414,13 +414,14 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 		for(auto i = armies[side]->Slots().begin(); i != armies[side]->Slots().end(); i++, k++)
 		{
 			std::vector<int> *formationVector = nullptr;
-			if(creatureBank)
+			if(armies[side]->formation == EArmyFormation::TIGHT )
-				formationVector = &creBankFormations[side][formationNo];
-			else if(armies[side]->formation)
 				formationVector = &tightFormations[side][formationNo];
 			else
 				formationVector = &looseFormations[side][formationNo];
 
+			if(creatureBank)
+				formationVector = &creBankFormations[side][formationNo];
+
 			BattleHex pos = (k < formationVector->size() ? formationVector->at(k) : 0);
 			if(creatureBank && i->second->type->isDoubleWide())
 				pos += side ? BattleHex::LEFT : BattleHex::RIGHT;

lib/mapObjects/CArmedInstance.cpp

@@ -23,14 +23,13 @@ void CArmedInstance::randomizeArmy(int type)
 {
 	for (auto & elem : stacks)
 	{
-		int & randID = elem.second->idRand;
+		if(elem.second->randomStack)
-		if(randID >= 0)
 		{
-			int level = randID / 2;
+			int level = elem.second->randomStack->level;
-			bool upgrade = randID % 2;
+			int upgrade = elem.second->randomStack->upgrade;
 			elem.second->setType((*VLC->townh)[type]->town->creatures[level][upgrade]);
 
-			randID = -1;
+			elem.second->randomStack = boost::none;
 		}
 		assert(elem.second->valid(false));
 		assert(elem.second->armyObj == this);

lib/mapObjects/CGHeroInstance.cpp

@@ -232,7 +232,7 @@ CGHeroInstance::CGHeroInstance():
 	portrait(UNINITIALIZED_PORTRAIT),
 	level(1),
 	exp(UNINITIALIZED_EXPERIENCE),
-	sex(std::numeric_limits<ui8>::max()),
+	gender(EHeroGender::DEFAULT),
 	lowestCreatureSpeed(0)
 {
 	setNodeType(HERO);
@@ -296,8 +296,8 @@ void CGHeroInstance::initHero(CRandomGenerator & rand)
 	if(secSkills.size() == 1 && secSkills[0] == std::pair<SecondarySkill,ui8>(SecondarySkill::DEFAULT, -1)) //set secondary skills to default
 		secSkills = type->secSkillsInit;
 
-	if (sex == 0xFF)//sex is default
+	if (gender == EHeroGender::DEFAULT)
-		sex = type->sex;
+		gender = type->gender;
 
 	setFormation(false);
 	if (!stacksCount()) //standard army//initial army
@@ -1468,7 +1468,7 @@ void CGHeroInstance::serializeCommonOptions(JsonSerializeFormat & handler)
 	}
 
 	handler.serializeString("name", nameCustom);
-	handler.serializeBool<ui8>("female", sex, 1, 0, 0xFF);
+	handler.serializeInt("gender", gender, 0);
 
 	{
 		const int legacyHeroes = VLC->settings()->getInteger(EGameSettings::TEXTS_HERO);
@@ -1619,8 +1619,10 @@ void CGHeroInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 			setHeroTypeName(typeName);
 	}
 
+	static const std::vector<std::string> FORMATIONS  =	{ "wide", "tight" };
+
 	CCreatureSet::serializeJson(handler, "army", 7);
-	handler.serializeBool<ui8>("tightFormation", formation, 1, 0, 0);
+	handler.serializeEnum("formation", formation, FORMATIONS);
 
 	{
 		static constexpr int NO_PATROLING = -1;

lib/mapObjects/CGHeroInstance.h

@@ -25,6 +25,7 @@ class CGTownInstance;
 class CMap;
 struct TerrainTile;
 struct TurnInfo;
+enum class EHeroGender : uint8_t;
 
 class CGHeroPlaceholder : public CGObjectInstance
 {
@@ -69,7 +70,7 @@ public:
 	si32 mana; // remaining spell points
 	std::vector<std::pair<SecondarySkill,ui8> > secSkills; //first - ID of skill, second - level of skill (1 - basic, 2 - adv., 3 - expert); if hero has ability (-1, -1) it meansthat it should have default secondary abilities
 	ui32 movement; //remaining movement points
-	ui8 sex;
+	EHeroGender gender;
 
 	std::string nameCustom;
 	std::string biographyCustom;
@@ -312,7 +313,7 @@ public:
 		h & mana;
 		h & secSkills;
 		h & movement;
-		h & sex;
+		h & gender;
 		h & inTownGarrison;
 		h & spells;
 		h & patrol;

lib/mapObjects/CGTownInstance.cpp

@@ -634,7 +634,7 @@ CGTownInstance::CGTownInstance():
 	builded(0),
 	destroyed(0),
 	identifier(0),
-	alignment(0xff)
+	alignmentToPlayer(PlayerColor::NEUTRAL)
 {
 	this->setNodeType(CBonusSystemNode::TOWN);
 }
@@ -1494,9 +1494,11 @@ void CGTownInstance::reset()
 
 void CGTownInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 {
+	static const std::vector<std::string> FORMATIONS  =	{ "wide", "tight" };
+
 	CGObjectInstance::serializeJsonOwner(handler);
 	CCreatureSet::serializeJson(handler, "army", 7);
-	handler.serializeBool<ui8>("tightFormation", formation, 1, 0, 0);
+	handler.serializeEnum("tightFormation", formation, FORMATIONS);
 	handler.serializeString("name", name);
 
 	{

lib/mapObjects/CGTownInstance.h

@@ -214,7 +214,7 @@ public:
 	si32 destroyed; //how many buildings has been destroyed this turn
 	ConstTransitivePtr<CGHeroInstance> garrisonHero, visitingHero;
 	ui32 identifier; //special identifier from h3m (only > RoE maps)
-	si32 alignment;
+	PlayerColor alignmentToPlayer; // if set to non-neutral, random town will have same faction as specified player
 	std::set<BuildingID> forbiddenBuildings;
 	std::set<BuildingID> builtBuildings;
 	std::set<BuildingID> overriddenBuildings; ///buildings which bonuses are overridden and should not be applied
@@ -239,7 +239,7 @@ public:
 		h & identifier;
 		h & garrisonHero;
 		h & visitingHero;
-		h & alignment;
+		h & alignmentToPlayer;
 		h & forbiddenBuildings;
 		h & builtBuildings;
 		h & bonusValue;

lib/mapObjects/MiscObjects.cpp

@@ -664,23 +664,13 @@ void CGMine::initObj(CRandomGenerator & rand)
 		auto * troglodytes = new CStackInstance(CreatureID::TROGLODYTES, howManyTroglodytes);
 		putStack(SlotID(0), troglodytes);
 
-		//after map reading tempOwner placeholds bitmask for allowed resources
+		assert(!abandonedMineResources.empty());
-		std::vector<GameResID> possibleResources;
+		producedResource = *RandomGeneratorUtil::nextItem(abandonedMineResources, rand);
-		for (int i = 0; i < PlayerColor::PLAYER_LIMIT_I; i++)
-			if(tempOwner.getNum() & 1<<i) //NOTE: reuse of tempOwner
-				possibleResources.push_back(GameResID(i));
-
-		assert(!possibleResources.empty());
-		producedResource = *RandomGeneratorUtil::nextItem(possibleResources, rand);
-		tempOwner = PlayerColor::NEUTRAL;
 	}
 	else
 	{
 		producedResource = GameResID(subID);
-		if(tempOwner >= PlayerColor::PLAYER_LIMIT)
-			tempOwner = PlayerColor::NEUTRAL;
 	}
-
 	producedQuantity = defaultResProduction();
 }
 
@@ -766,31 +756,20 @@ void CGMine::serializeJsonOptions(JsonSerializeFormat & handler)
 		if(handler.saving)
 		{
 			JsonNode node(JsonNode::JsonType::DATA_VECTOR);
-			for(int i = 0; i < PlayerColor::PLAYER_LIMIT_I; i++)
+			for(auto const & resID : abandonedMineResources)
-			{
-				if(tempOwner.getNum() & 1<<i)
 			{
 				JsonNode one(JsonNode::JsonType::DATA_STRING);
-					one.String() = GameConstants::RESOURCE_NAMES[i];
+				one.String() = GameConstants::RESOURCE_NAMES[resID];
 				node.Vector().push_back(one);
 			}
-			}
 			handler.serializeRaw("possibleResources", node, boost::none);
 		}
 		else
 		{
 			auto guard = handler.enterArray("possibleResources");
 			const JsonNode & node = handler.getCurrent();
-			std::set<int> possibleResources;
+			std::set<int> abandonedMineResources;
 
-			if(node.getType() != JsonNode::JsonType::DATA_VECTOR || node.Vector().empty())
-			{
-				//assume all allowed
-				for(int i = static_cast<int>(EGameResID::WOOD); i < static_cast<int>(EGameResID::GOLD); i++)
-					possibleResources.insert(i);
-			}
-			else
-			{
 			auto names = node.convertTo<std::vector<std::string>>();
 
 			for(const std::string & s : names)
@@ -799,14 +778,7 @@ void CGMine::serializeJsonOptions(JsonSerializeFormat & handler)
 				if(raw_res < 0)
 					logGlobal->error("Invalid resource name: %s", s);
 				else
-						possibleResources.insert(raw_res);
+					abandonedMineResources.insert(raw_res);
-				}
-
-				int tmp = 0;
-
-				for(int r : possibleResources)
-					tmp |=  (1<<r);
-				tempOwner = PlayerColor(tmp);
 			}
 		}
 	}
@@ -1427,7 +1399,7 @@ void CGWitchHut::initObj(CRandomGenerator & rand)
 		// Necromancy can't be learned on random maps
 		for(int i = 0; i < VLC->skillh->size(); i++)
 			if(VLC->skillh->getByIndex(i)->getId() != SecondarySkill::NECROMANCY)
-				allowedAbilities.push_back(i);
+				allowedAbilities.insert(i);
 	}
 	ability = *RandomGeneratorUtil::nextItem(allowedAbilities, rand);
 }
@@ -1503,7 +1475,7 @@ void CGWitchHut::serializeJsonOptions(JsonSerializeFormat & handler)
 		allowedAbilities.clear();
 		for(si32 i = 0; i < skillCount; ++i)
 			if(temp[i])
-				allowedAbilities.push_back(i);
+				allowedAbilities.insert(i);
 	}
 }
 

lib/mapObjects/MiscObjects.h

@@ -131,7 +131,7 @@ protected:
 class DLL_LINKAGE CGWitchHut : public CTeamVisited
 {
 public:
-	std::vector<si32> allowedAbilities;
+	std::set<si32> allowedAbilities;
 	ui32 ability;
 
 	std::string getHoverText(PlayerColor player) const override;
@@ -265,6 +265,7 @@ class DLL_LINKAGE CGMine : public CArmedInstance
 public:
 	GameResID producedResource;
 	ui32 producedQuantity;
+	std::set<GameResID> abandonedMineResources;
 
 private:
 	void onHeroVisit(const CGHeroInstance * h) const override;
@@ -285,6 +286,7 @@ public:
 		h & static_cast<CArmedInstance&>(*this);
 		h & producedResource;
 		h & producedQuantity;
+		h & abandonedMineResources;
 	}
 	ui32 defaultResProduction() const;
 

lib/mapping/CMap.cpp

@@ -35,7 +35,7 @@ SHeroName::SHeroName() : heroId(-1)
 
 PlayerInfo::PlayerInfo(): canHumanPlay(false), canComputerPlay(false),
 	aiTactic(EAiTactic::RANDOM), isFactionRandom(false), hasRandomHero(false), mainCustomHeroPortrait(-1), mainCustomHeroId(-1), hasMainTown(false),
-	generateHeroAtMainTown(false), posOfMainTown(-1), team(TeamID::NO_TEAM), /* following are unused */ generateHero(false), p7(0), powerPlaceholders(-1)
+	generateHeroAtMainTown(false), posOfMainTown(-1), team(TeamID::NO_TEAM)
 {
 	allowedFactions = VLC->townh->getAllowedFactions();
 }

lib/mapping/CMap.h

@@ -86,17 +86,9 @@ struct DLL_LINKAGE PlayerInfo
 	int3 posOfMainTown;
 	TeamID team; /// The default value NO_TEAM
 
-
-	bool generateHero; /// Unused.
-	si32 p7; /// Unknown and unused.
-	/// Unused. Count of hero placeholders containing hero type.
-	/// WARNING: powerPlaceholders sometimes gives false 0 (eg. even if there is one placeholder), maybe different meaning ???
-	ui8 powerPlaceholders;
-
 	template <typename Handler>
 	void serialize(Handler & h, const int version)
 	{
-		h & p7;
 		h & hasRandomHero;
 		h & mainCustomHeroId;
 		h & canHumanPlay;
@@ -111,7 +103,6 @@ struct DLL_LINKAGE PlayerInfo
 		h & generateHeroAtMainTown;
 		h & posOfMainTown;
 		h & team;
-		h & generateHero;
 		h & mainHeroInstance;
 	}
 };
@@ -259,20 +250,17 @@ struct DLL_LINKAGE DisposedHero
 	}
 };
 
-namespace EMapFormat
+enum class EMapFormat: uint8_t
-{
-enum EMapFormat: ui8
 {
 	INVALID = 0,
 	//    HEX     DEC
 	ROE  = 0x0e, // 14
 	AB   = 0x15, // 21
 	SOD  = 0x1c, // 28
-// HOTA = 0x1e ... 0x20 // 28 ... 30
+	HOTA = 0x1e, // 30
 	WOG  = 0x33, // 51
 	VCMI = 0xF0
 };
-}
 
 /// The map header holds information about loss/victory condition,map format, version, players, height, width,...
 class DLL_LINKAGE CMapHeader
@@ -293,7 +281,7 @@ public:
 
 	ui8 levels() const;
 
-	EMapFormat::EMapFormat version; /// The default value is EMapFormat::SOD.
+	EMapFormat version; /// The default value is EMapFormat::SOD.
 	si32 height; /// The default value is 72.
 	si32 width; /// The default value is 72.
 	bool twoLevel; /// The default value is true.

lib/mapping/CMapService.cpp

@@ -120,10 +120,10 @@ std::unique_ptr<IMapLoader> CMapService::getMapLoader(std::unique_ptr<CInputStre
 			case 0x00088B1F:
 				stream = std::unique_ptr<CInputStream>(new CCompressedStream(std::move(stream), true));
 				return std::unique_ptr<IMapLoader>(new CMapLoaderH3M(mapName, modName, encoding, stream.get()));
-			case EMapFormat::WOG :
+			case static_cast<int>(EMapFormat::WOG) :
-			case EMapFormat::AB  :
+			case static_cast<int>(EMapFormat::AB)  :
-			case EMapFormat::ROE :
+			case static_cast<int>(EMapFormat::ROE) :
-			case EMapFormat::SOD :
+			case static_cast<int>(EMapFormat::SOD) :
 				return std::unique_ptr<IMapLoader>(new CMapLoaderH3M(mapName, modName, encoding, stream.get()));
 			default :
 				throw std::runtime_error("Unknown map format");

lib/mapping/MapFeaturesH3M.cpp

@@ -0,0 +1,116 @@
+/*
+ * MapFeaturesH3M.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#include "StdInc.h"
+#include "MapFeaturesH3M.h"
+#include "CMap.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+MapFormatFeaturesH3M MapFormatFeaturesH3M::find(EMapFormat format)
+{
+	switch (format)
+	{
+		case EMapFormat::ROE:  return getFeaturesROE();
+		case EMapFormat::AB:   return getFeaturesAB();
+		case EMapFormat::SOD:  return getFeaturesSOD();
+		case EMapFormat::WOG:  return getFeaturesWOG();
+		case EMapFormat::HOTA: return getFeaturesHOTA();
+		default:
+			throw std::runtime_error("Invalid map format!");
+	}
+}
+
+MapFormatFeaturesH3M MapFormatFeaturesH3M::getFeaturesROE()
+{
+	MapFormatFeaturesH3M result;
+	result.levelROE = true;
+
+	result.factionsBytes = 1;
+	result.heroesBytes = 16;
+	result.artifactsBytes = 16;
+	result.skillsBytes = 4;
+	result.resourcesBytes = 4;
+	result.spellsBytes = 9;
+	result.buildingsBytes = 6;
+
+	result.factionsCount = 8;
+	result.heroesCount = 128;
+	result.heroesPortraitsCount = 128;
+	result.artifactsCount = 127;
+	result.resourcesCount = 7;
+	result.creaturesCount = 118;
+	result.spellsCount = 70;
+	result.skillsCount = 28;
+	result.terrainsCount = 10;
+	result.artifactSlotsCount = 18;
+	result.buildingsCount = 40;
+
+	result.heroIdentifierInvalid = 0xff;
+	result.artifactIdentifierInvalid = 0xff;
+	result.creatureIdentifierInvalid = 0xff;
+	result.spellIdentifierInvalid = 0xff;
+
+	return result;
+}
+
+MapFormatFeaturesH3M MapFormatFeaturesH3M::getFeaturesAB()
+{
+	MapFormatFeaturesH3M result = getFeaturesROE();
+	result.levelAB = true;
+
+	result.factionsBytes = 2; // + Conflux
+	result.factionsCount = 9;
+
+	result.creaturesCount = 144; // + Conflux and new neutrals
+
+	result.heroesCount = 156; // + Conflux and campaign heroes
+	result.heroesPortraitsCount = 163;
+	result.heroesBytes = 20;
+
+	result.artifactsCount = 129; // + Armaggedon Blade and Vial of Dragon Blood
+	result.artifactsBytes = 17;
+
+	result.artifactIdentifierInvalid = 0xffff; // Now uses 2 bytes / object
+	result.creatureIdentifierInvalid = 0xffff; // Now uses 2 bytes / object
+
+	return result;
+}
+
+MapFormatFeaturesH3M MapFormatFeaturesH3M::getFeaturesSOD()
+{
+	MapFormatFeaturesH3M result = getFeaturesAB();
+	result.levelSOD = true;
+
+	result.artifactsCount = 141; // + Combined artifacts
+	result.artifactsBytes = 18;
+
+	result.artifactSlotsCount = 19; // + MISC_5 slot
+
+	return result;
+}
+
+MapFormatFeaturesH3M MapFormatFeaturesH3M::getFeaturesWOG()
+{
+	MapFormatFeaturesH3M result = getFeaturesSOD();
+	result.levelWOG = true;
+
+	return result;
+}
+
+MapFormatFeaturesH3M MapFormatFeaturesH3M::getFeaturesHOTA()
+{
+	MapFormatFeaturesH3M result = getFeaturesSOD();
+	result.levelHOTA = true;
+
+	return result;
+}
+
+VCMI_LIB_NAMESPACE_END

lib/mapping/MapFeaturesH3M.h

@@ -0,0 +1,69 @@
+/*
+ * MapFeaturesH3M.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#pragma once
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+enum class EMapFormat : uint8_t;
+
+struct MapFormatFeaturesH3M
+{
+public:
+	static MapFormatFeaturesH3M find(EMapFormat format);
+	static MapFormatFeaturesH3M getFeaturesROE();
+	static MapFormatFeaturesH3M getFeaturesAB();
+	static MapFormatFeaturesH3M getFeaturesSOD();
+	static MapFormatFeaturesH3M getFeaturesWOG();
+	static MapFormatFeaturesH3M getFeaturesHOTA();
+
+	MapFormatFeaturesH3M() = default;
+
+	// number of bytes in bitmask of appropriate type
+
+	int factionsBytes;
+	int heroesBytes;
+	int artifactsBytes;
+	int resourcesBytes;
+	int skillsBytes;
+	int spellsBytes;
+	int buildingsBytes;
+
+	// total number of elements of appropriate type
+
+	int factionsCount;
+	int heroesCount;
+	int heroesPortraitsCount;
+	int artifactsCount;
+	int resourcesCount;
+	int creaturesCount;
+	int spellsCount;
+	int skillsCount;
+	int terrainsCount;
+	int artifactSlotsCount;
+	int buildingsCount;
+
+	// identifier that should be treated as "invalid", usually - '-1'
+
+	int heroIdentifierInvalid;
+	int artifactIdentifierInvalid;
+	int creatureIdentifierInvalid;
+	int spellIdentifierInvalid;
+
+	// features from which map format are available
+
+	bool levelROE = false;
+	bool levelAB = false;
+	bool levelSOD = false;
+	bool levelWOG = false;
+	bool levelHOTA = false;
+};
+
+VCMI_LIB_NAMESPACE_END

lib/mapping/MapFormatH3M.h

@@ -11,17 +11,12 @@
 #pragma once
 
 #include "CMapService.h"
-#include "../GameConstants.h"
+#include "MapFeaturesH3M.h"
-#include "../ResourceSet.h"
-#include "../mapObjects/ObjectTemplate.h"
-
-#include "../int3.h"
-
 
 VCMI_LIB_NAMESPACE_BEGIN
 
 class CGHeroInstance;
-class CBinaryReader;
+class MapReaderH3M;
 class CArtifactInstance;
 class CGObjectInstance;
 class CGSeerHut;
@@ -31,6 +26,11 @@ class CCreatureSet;
 class CInputStream;
 class TextIdentifier;
 
+class ObjectInstanceID;
+class BuildingID;
+class ObjectTemplate;
+class SpellID;
+class int3;
 
 class DLL_LINKAGE CMapLoaderH3M : public IMapLoader
 {
@@ -61,9 +61,6 @@ public:
 	 */
 	std::unique_ptr<CMapHeader> loadMapHeader() override;
 
-	/** true if you want to enable the map loader profiler to see how long a specific part took; default=false */
-	static const bool IS_PROFILING_ENABLED;
-
 private:
 	/**
 	 * Initializes the map object from parsing the input buffer.
@@ -209,42 +206,18 @@ private:
 	*/
 	void readMessageAndGuards(std::string & message, CCreatureSet * guards, const int3 & position);
 
-	void readSpells(std::set<SpellID> & dest);
-
-	void readResourses(TResources& resources);
-
-	template <class Indenifier>
-	void readBitmask(std::set<Indenifier> &dest, const int byteCount, const int limit, bool negate = true);
-
-	/** Reads bitmask to boolean vector
-	* @param dest destination vector, shall be filed with "true" values
-	* @param byteCount size in bytes of bimask
-	* @param limit max count of vector elements to alter
-	* @param negate if true then set bit in mask means clear flag in vertor
-	*/
-	void readBitmask(std::vector<bool> & dest, const int byteCount, const int limit, bool negate = true);
-
-	/**
-	 * Reverses the input argument.
-	 *
-	 * @param arg the input argument
-	 * @return the reversed 8-bit integer
-	 */
-	ui8 reverse(ui8 arg) const;
-
-	/**
-	* Helper to read map position
-	*/
-	int3 readInt3();
-
 	/// reads string from input stream and converts it to unicode
 	std::string readBasicString();
 
 	/// reads string from input stream, converts it to unicode and attempts to translate it
 	std::string readLocalizedString(const TextIdentifier & identifier);
 
+	void readSpells(std::set<SpellID> & dest);
+
 	void afterRead();
 
+	MapFormatFeaturesH3M features;
+
 	/** List of templates loaded from the map, used on later stage to create
 	 *  objects but not needed for fully functional CMap */
 	std::vector<std::shared_ptr<const ObjectTemplate>> templates;
@@ -257,7 +230,7 @@ private:
 	 * (when loading a map then the mapHeader ptr points to the same object)
 	 */
 	std::unique_ptr<CMapHeader> mapHeader;
-	std::unique_ptr<CBinaryReader> reader;
+	std::unique_ptr<MapReaderH3M> reader;
 	CInputStream * inputStream;
 
 	std::string mapName;

lib/mapping/MapReaderH3M.cpp

@@ -0,0 +1,244 @@
+/*
+ * MapReaderH3M.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#include "StdInc.h"
+#include "MapReaderH3M.h"
+#include "CMap.h"
+#include "../filesystem/CBinaryReader.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+MapReaderH3M::MapReaderH3M(CInputStream * stream)
+	: reader(std::make_unique<CBinaryReader>(stream))
+{
+}
+
+void MapReaderH3M::setFormatLevel(EMapFormat newFormat)
+{
+	features = MapFormatFeaturesH3M::find(newFormat);
+}
+
+ArtifactID MapReaderH3M::readArtifact()
+{
+	ArtifactID result;
+
+	if (features.levelAB)
+		result = ArtifactID(reader->readUInt16());
+	else
+		result = ArtifactID(reader->readUInt8());
+
+	if (result == features.artifactIdentifierInvalid)
+		return ArtifactID::NONE;
+
+	assert(result < features.artifactsCount);
+	return result;
+}
+
+HeroTypeID MapReaderH3M::readHero()
+{
+	HeroTypeID result (reader->readUInt8());
+
+	if (result.getNum() == features.heroIdentifierInvalid)
+		return HeroTypeID(-1);
+
+	assert(result.getNum() < features.heroesPortraitsCount);
+	return result;
+}
+
+CreatureID MapReaderH3M::readCreature()
+{
+	CreatureID result;
+
+	if (features.levelAB)
+		result = CreatureID(reader->readUInt16());
+	else
+		result = CreatureID(reader->readUInt8());
+
+	if (result == features.creatureIdentifierInvalid)
+		return CreatureID::NONE;
+
+	if(result > features.creaturesCount)
+	{
+		// this may be random creature in army/town, to be randomized later
+		CreatureID randomIndex (result.getNum() - features.creatureIdentifierInvalid - 1);
+		assert(randomIndex < CreatureID::NONE);
+		assert(randomIndex > -16);
+		return randomIndex;
+	}
+
+	return result;
+
+
+}
+
+TerrainId MapReaderH3M::readTerrain()
+{
+	TerrainId result(readUInt8());
+	assert (result.getNum() < features.terrainsCount);
+	return result;
+}
+
+RoadId MapReaderH3M::readRoad()
+{
+	RoadId result(readUInt8());
+	assert (result < Road::ORIGINAL_ROAD_COUNT);
+	return result;
+}
+
+RiverId MapReaderH3M::readRiver()
+{
+	RiverId result(readUInt8());
+	assert (result < River::ORIGINAL_RIVER_COUNT);
+	return result;
+
+}
+
+SecondarySkill MapReaderH3M::readSkill()
+{
+	SecondarySkill result(readUInt8());
+	assert (result < features.skillsCount);
+	return result;
+}
+
+SpellID MapReaderH3M::readSpell()
+{
+	SpellID result(readUInt8());
+	if (result == features.spellIdentifierInvalid)
+		return SpellID::NONE;
+	if (result == features.spellIdentifierInvalid - 1)
+		return SpellID::PRESET;
+
+	assert (result < features.spellsCount);
+	return result;
+}
+
+SpellID MapReaderH3M::readSpell32()
+{
+	SpellID result(readUInt32());
+	if (result == features.spellIdentifierInvalid)
+		return SpellID::NONE;
+	assert (result < features.spellsCount);
+	return result;
+}
+
+PlayerColor MapReaderH3M::readPlayer()
+{
+	PlayerColor result(readUInt8());
+	assert (result < PlayerColor::PLAYER_LIMIT || result == PlayerColor::NEUTRAL);
+	return result;
+}
+
+PlayerColor MapReaderH3M::readPlayer32()
+{
+	PlayerColor result(readUInt32());
+
+	assert (result < PlayerColor::PLAYER_LIMIT || result == PlayerColor::NEUTRAL);
+	return result;
+}
+
+void MapReaderH3M::readBitmask(std::vector<bool> & dest, const int byteCount, const int limit, bool negate)
+{
+	for(int byte = 0; byte < byteCount; ++byte)
+	{
+		const ui8 mask = reader->readUInt8();
+		for(int bit = 0; bit < 8; ++bit)
+		{
+			if(byte * 8 + bit < limit)
+			{
+				const bool flag = mask & (1 << bit);
+				if((negate && flag) || (!negate && !flag)) // FIXME: check PR388
+					dest[byte * 8 + bit] = false;
+			}
+		}
+	}
+}
+
+int3 MapReaderH3M::readInt3()
+{
+	int3 p;
+	p.x = reader->readUInt8();
+	p.y = reader->readUInt8();
+	p.z = reader->readUInt8();
+	return p;
+}
+
+void MapReaderH3M::skipUnused(size_t amount)
+{
+	reader->skip(amount);
+}
+
+void MapReaderH3M::skipZero(size_t amount)
+{
+#ifdef NDEBUG
+	skipUnused(amount);
+#else
+	for (size_t i = 0; i < amount; ++i)
+	{
+		uint8_t value = reader->readUInt8();
+		assert(value == 0);
+	}
+#endif
+}
+
+void MapReaderH3M::readResourses(TResources& resources)
+{
+	for(int x = 0; x < features.resourcesCount; ++x)
+		resources[x] = reader->readUInt32();
+}
+
+bool MapReaderH3M::readBool()
+{
+	uint8_t result = readUInt8();
+	assert(result == 0 || result == 1);
+
+	return result != 0;
+}
+
+ui8  MapReaderH3M::readUInt8()
+{
+	return reader->readUInt8();
+}
+
+si8  MapReaderH3M::readInt8()
+{
+	return reader->readInt8();
+}
+
+ui16 MapReaderH3M::readUInt16()
+{
+	return reader->readUInt16();
+}
+
+si16 MapReaderH3M::readInt16()
+{
+	return reader->readInt16();
+}
+
+ui32 MapReaderH3M::readUInt32()
+{
+	return reader->readUInt32();
+}
+
+si32 MapReaderH3M::readInt32()
+{
+	return reader->readInt32();
+}
+
+std::string MapReaderH3M::readBaseString()
+{
+	return reader->readBaseString();
+}
+
+CBinaryReader & MapReaderH3M::getInternalReader()
+{
+	return *reader;
+}
+
+VCMI_LIB_NAMESPACE_END

lib/mapping/MapReaderH3M.h

@@ -0,0 +1,92 @@
+/*
+ * MapReaderH3M.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#pragma once
+
+#include "../ResourceSet.h"
+#include "../GameConstants.h"
+#include "MapFeaturesH3M.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+class CBinaryReader;
+class CInputStream;
+struct MapFormatFeaturesH3M;
+class int3;
+enum class EMapFormat : uint8_t;
+
+class MapReaderH3M
+{
+public:
+	explicit MapReaderH3M(CInputStream * stream);
+
+	void setFormatLevel(EMapFormat format);
+
+	ArtifactID readArtifact();
+	CreatureID readCreature();
+	HeroTypeID readHero();
+	TerrainId readTerrain();
+	RoadId readRoad();
+	RiverId readRiver();
+	SecondarySkill readSkill();
+	SpellID readSpell();
+	SpellID readSpell32();
+	PlayerColor readPlayer();
+	PlayerColor readPlayer32();
+
+	template <class Identifier>
+	void readBitmask(std::set<Identifier> &dest, const int byteCount, const int limit, bool negate = true)
+	{
+		std::vector<bool> temp;
+		temp.resize(limit,true);
+		readBitmask(temp, byteCount, limit, negate);
+
+		for(int i = 0; i< std::min(temp.size(), static_cast<size_t>(limit)); i++)
+			if(temp[i])
+				dest.insert(static_cast<Identifier>(i));
+	}
+
+	/** Reads bitmask to boolean vector
+	* @param dest destination vector, shall be filed with "true" values
+	* @param byteCount size in bytes of bimask
+	* @param limit max count of vector elements to alter
+	* @param negate if true then set bit in mask means clear flag in vertor
+	*/
+	void readBitmask(std::vector<bool> & dest, int byteCount, int limit, bool negate = true);
+
+	/**
+	* Helper to read map position
+	*/
+	int3 readInt3();
+
+	void skipUnused(size_t amount);
+	void skipZero(size_t amount);
+
+	void readResourses(TResources& resources);
+
+	bool readBool();
+
+	ui8  readUInt8();
+	si8  readInt8();
+	ui16 readUInt16();
+	si16 readInt16();
+	ui32 readUInt32();
+	si32 readInt32();
+
+	std::string readBaseString();
+
+	CBinaryReader & getInternalReader();
+private:
+	MapFormatFeaturesH3M features;
+
+	std::unique_ptr<CBinaryReader> reader;
+};
+
+VCMI_LIB_NAMESPACE_END

lib/spells/effects/Summon.cpp

@@ -67,7 +67,7 @@ bool Summon::applicable(Problem & problem, const Mechanics * m) const
 
 				text.addReplacement(MetaString::CRE_PL_NAMES, elemental->creatureIndex());
 
-				if(caster->type->sex)
+				if(caster->type->gender == EHeroGender::FEMALE)
 					text.addReplacement(MetaString::GENERAL_TXT, 540);
 				else
 					text.addReplacement(MetaString::GENERAL_TXT, 539);

mapeditor/inspector/armywidget.cpp

@@ -68,7 +68,7 @@ void ArmyWidget::obtainData()
 		}
 	}
 	
-	if(army.formation)
+	if(army.formation == EArmyFormation::TIGHT)
 		ui->formationTight->setChecked(true);
 	else
 		ui->formationWide->setChecked(true);

mapeditor/inspector/inspector.cpp

@@ -132,7 +132,7 @@ void Initializer::initialize(CGHeroInstance * o)
 	if(!o->type)
 		o->type = VLC->heroh->objects.at(o->subID);
 	
-	o->sex = o->type->sex;
+	o->gender = o->type->gender;
 	o->portrait = o->type->imageIndex;
 	o->randomizeArmy(o->type->heroClass->faction);
 }
@@ -241,7 +241,7 @@ void Inspector::updateProperties(CGHeroInstance * o)
 	{ //Sex
 		auto * delegate = new InspectorDelegate;
 		delegate->options << "MALE" << "FEMALE";
-		addProperty<std::string>("Sex", (o->sex ? "FEMALE" : "MALE"), delegate , false);
+		addProperty<std::string>("Gender", (o->gender == EHeroGender::FEMALE ? "FEMALE" : "MALE"), delegate , false);
 	}
 	addProperty("Name", o->nameCustom, false);
 	addProperty("Biography", o->biographyCustom, new MessageDelegate, false);
@@ -549,8 +549,8 @@ void Inspector::setProperty(CGHeroInstance * o, const QString & key, const QVari
 {
 	if(!o) return;
 	
-	if(key == "Sex")
+	if(key == "Gender")
-		o->sex = value.toString() == "MALE" ? 0 : 1;
+		o->gender = value.toString() == "MALE" ? EHeroGender::MALE : EHeroGender::FEMALE;
 	
 	if(key == "Name")
 		o->nameCustom = value.toString().toStdString();
@@ -565,7 +565,7 @@ void Inspector::setProperty(CGHeroInstance * o, const QString & key, const QVari
 			if(t->getNameTranslated() == value.toString().toStdString())
 				o->type = t.get();
 		}
-		o->sex = o->type->sex;
+		o->gender = o->type->gender;
 		o->portrait = o->type->imageIndex;
 		o->randomizeArmy(o->type->heroClass->faction);
 		updateProperties(); //updating other properties after change