From 9e5e7d95c356d364f8c2b4118e37381aa3ea05ce Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Sun, 4 Feb 2024 21:22:51 +0200 Subject: [PATCH 01/10] Workaround for broken save compatibility --- lib/registerTypes/RegisterTypesMapObjects.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/registerTypes/RegisterTypesMapObjects.h b/lib/registerTypes/RegisterTypesMapObjects.h index ddaac348a..5e8d96f32 100644 --- a/lib/registerTypes/RegisterTypesMapObjects.h +++ b/lib/registerTypes/RegisterTypesMapObjects.h @@ -54,7 +54,6 @@ void registerTypesMapObjects(Serializer &s) s.template registerType(); s.template registerType(); s.template registerType(); - s.template registerType(); s.template registerType(); s.template registerType(); s.template registerType(); s.template registerType(); @@ -133,6 +132,8 @@ void registerTypesMapObjects(Serializer &s) //s.template registerType(); s.template registerType(); + + s.template registerType(); } VCMI_LIB_NAMESPACE_END From a97d1d93771139a500bfa9e90d4a3e30d9b9941a Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 5 Feb 2024 21:07:01 +0200 Subject: [PATCH 02/10] Fix crash on broken ENCHANTED bonus --- server/battles/BattleFlowProcessor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/battles/BattleFlowProcessor.cpp b/server/battles/BattleFlowProcessor.cpp index a27be3606..94d585803 100644 --- a/server/battles/BattleFlowProcessor.cpp +++ b/server/battles/BattleFlowProcessor.cpp @@ -584,10 +584,10 @@ void BattleFlowProcessor::stackEnchantedTrigger(const CBattleInfoCallback & batt auto bl = *(st->getBonuses(Selector::type()(BonusType::ENCHANTED))); for(auto b : bl) { - const CSpell * sp = b->subtype.as().toSpell(); - if(!sp) + if (!b->subtype.as().hasValue()) continue; + const CSpell * sp = b->subtype.as().toSpell(); const int32_t val = bl.valOfBonuses(Selector::typeSubtype(b->type, b->subtype)); const int32_t level = ((val > 3) ? (val - 3) : val); From 342e6daebda2c91296499424c764d637f821d3b2 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 5 Feb 2024 21:07:50 +0200 Subject: [PATCH 03/10] Fix copy-paste error --- lib/constants/EntityIdentifiers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/constants/EntityIdentifiers.h b/lib/constants/EntityIdentifiers.h index 7bebcdf66..070d26492 100644 --- a/lib/constants/EntityIdentifiers.h +++ b/lib/constants/EntityIdentifiers.h @@ -330,7 +330,7 @@ public: static BuildingID FORT_LEVEL(unsigned int level) { assert(level < 3); - return BuildingID(Type::TOWN_HALL + level); + return BuildingID(Type::FORT + level); } static std::string encode(int32_t index); From 4af4d1a75ea0a31576f46573cdf45d07d8e45f0e Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 5 Feb 2024 21:11:00 +0200 Subject: [PATCH 04/10] Remove excessive logging --- lib/mapping/CMapHeader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mapping/CMapHeader.cpp b/lib/mapping/CMapHeader.cpp index 8144b857f..277a29c1c 100644 --- a/lib/mapping/CMapHeader.cpp +++ b/lib/mapping/CMapHeader.cpp @@ -149,7 +149,7 @@ void CMapHeader::registerMapStrings() if(maxStrings == 0 || mapLanguages.empty()) { - logGlobal->info("Map %s doesn't have any supported translation", name.toString()); + logGlobal->trace("Map %s doesn't have any supported translation", name.toString()); return; } From 87059be67bba0e8ebcf214fdb1b04a65a8289b68 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 5 Feb 2024 21:27:55 +0200 Subject: [PATCH 05/10] Added range checks to values read from h3m. Fixes reading of morale/luck values (-3..3) as unsigned leading to overflow. --- client/lobby/CSelectionBase.cpp | 2 +- client/lobby/RandomMapTab.cpp | 2 +- lib/CHeroHandler.h | 4 +- lib/constants/Enumerations.h | 8 +- lib/mapObjects/CGHeroInstance.h | 2 +- lib/mapping/CMapDefines.h | 4 +- lib/mapping/CMapHeader.cpp | 2 +- lib/mapping/CMapHeader.h | 11 ++- lib/mapping/MapFormatH3M.cpp | 160 ++++++++++++-------------------- lib/mapping/MapFormatH3M.h | 46 ++++++++- lib/mapping/MapReaderH3M.cpp | 35 ++++--- lib/mapping/MapReaderH3M.h | 17 ++-- lib/rmg/CMapGenerator.cpp | 2 +- 13 files changed, 154 insertions(+), 141 deletions(-) diff --git a/client/lobby/CSelectionBase.cpp b/client/lobby/CSelectionBase.cpp index f35ce9e18..c8ad4276c 100644 --- a/client/lobby/CSelectionBase.cpp +++ b/client/lobby/CSelectionBase.cpp @@ -229,7 +229,7 @@ void InfoCard::changeSelection() iconsLossCondition->setFrame(header->defeatIconIndex); labelLossConditionText->setText(header->defeatMessage.toString()); flagbox->recreate(); - labelDifficulty->setText(CGI->generaltexth->arraytxt[142 + mapInfo->mapHeader->difficulty]); + labelDifficulty->setText(CGI->generaltexth->arraytxt[142 + vstd::to_underlying(mapInfo->mapHeader->difficulty)]); iconDifficulty->setSelected(SEL->getCurrentDifficulty()); if(SEL->screenType == ESelectionScreen::loadGame || SEL->screenType == ESelectionScreen::saveGame) for(auto & button : iconDifficulty->buttons) diff --git a/client/lobby/RandomMapTab.cpp b/client/lobby/RandomMapTab.cpp index 8696ba1fa..070319766 100644 --- a/client/lobby/RandomMapTab.cpp +++ b/client/lobby/RandomMapTab.cpp @@ -177,7 +177,7 @@ void RandomMapTab::updateMapInfoByHost() mapInfo->mapHeader->version = EMapFormat::VCMI; mapInfo->mapHeader->name.appendLocalString(EMetaText::GENERAL_TXT, 740); mapInfo->mapHeader->description.appendLocalString(EMetaText::GENERAL_TXT, 741); - mapInfo->mapHeader->difficulty = 1; // Normal + mapInfo->mapHeader->difficulty = EMapDifficulty::NORMAL; mapInfo->mapHeader->height = mapGenOptions->getHeight(); mapInfo->mapHeader->width = mapGenOptions->getWidth(); mapInfo->mapHeader->twoLevel = mapGenOptions->getHasTwoLevels(); diff --git a/lib/CHeroHandler.h b/lib/CHeroHandler.h index 1a3219f8c..1b066d029 100644 --- a/lib/CHeroHandler.h +++ b/lib/CHeroHandler.h @@ -31,11 +31,11 @@ class CRandomGenerator; class JsonSerializeFormat; class BattleField; -enum class EHeroGender : uint8_t +enum class EHeroGender : int8_t { + DEFAULT = -1, // from h3m, instance has same gender as hero type MALE = 0, FEMALE = 1, - DEFAULT = 0xff // from h3m, instance has same gender as hero type }; class DLL_LINKAGE CHero : public HeroType diff --git a/lib/constants/Enumerations.h b/lib/constants/Enumerations.h index 67ddc1b4a..9c7b93f0e 100644 --- a/lib/constants/Enumerations.h +++ b/lib/constants/Enumerations.h @@ -64,10 +64,10 @@ enum class EMarketMode : int8_t enum class EAiTactic : int8_t { NONE = -1, - RANDOM, - WARRIOR, - BUILDER, - EXPLORER + RANDOM = 0, + WARRIOR = 1, + BUILDER = 2, + EXPLORER = 3 }; enum class EBuildingState : int8_t diff --git a/lib/mapObjects/CGHeroInstance.h b/lib/mapObjects/CGHeroInstance.h index 61ca63750..5f6683a7f 100644 --- a/lib/mapObjects/CGHeroInstance.h +++ b/lib/mapObjects/CGHeroInstance.h @@ -23,7 +23,7 @@ class CGTownInstance; class CMap; struct TerrainTile; struct TurnInfo; -enum class EHeroGender : uint8_t; +enum class EHeroGender : int8_t; class DLL_LINKAGE CGHeroPlaceholder : public CGObjectInstance { diff --git a/lib/mapping/CMapDefines.h b/lib/mapping/CMapDefines.h index 27ad56a32..e1032aff7 100644 --- a/lib/mapping/CMapDefines.h +++ b/lib/mapping/CMapDefines.h @@ -37,8 +37,8 @@ public: MetaString message; TResources resources; ui8 players; // affected players, bit field? - ui8 humanAffected; - ui8 computerAffected; + bool humanAffected; + bool computerAffected; ui32 firstOccurence; ui32 nextOccurence; /// specifies after how many days the event will occur the next time; 0 if event occurs only one time diff --git a/lib/mapping/CMapHeader.cpp b/lib/mapping/CMapHeader.cpp index 277a29c1c..c67940ac5 100644 --- a/lib/mapping/CMapHeader.cpp +++ b/lib/mapping/CMapHeader.cpp @@ -117,7 +117,7 @@ void CMapHeader::setupEvents() } CMapHeader::CMapHeader() : version(EMapFormat::VCMI), height(72), width(72), - twoLevel(true), difficulty(1), levelLimit(0), howManyTeams(0), areAnyPlayers(false) + twoLevel(true), difficulty(EMapDifficulty::NORMAL), levelLimit(0), howManyTeams(0), areAnyPlayers(false) { setupEvents(); allowedHeroes = VLC->heroh->getDefaultAllowed(); diff --git a/lib/mapping/CMapHeader.h b/lib/mapping/CMapHeader.h index c343fcab7..3be7fc58e 100644 --- a/lib/mapping/CMapHeader.h +++ b/lib/mapping/CMapHeader.h @@ -191,6 +191,15 @@ struct DLL_LINKAGE TriggeredEvent } }; +enum class EMapDifficulty : uint8_t +{ + EASY = 0, + NORMAL = 1, + HARD = 2, + EXPERT = 3, + IMPOSSIBLE = 4 +}; + /// The map header holds information about loss/victory condition,map format, version, players, height, width,... class DLL_LINKAGE CMapHeader { @@ -218,7 +227,7 @@ public: bool twoLevel; /// The default value is true. MetaString name; MetaString description; - ui8 difficulty; /// The default value is 1 representing a normal map difficulty. + EMapDifficulty difficulty; /// Specifies the maximum level to reach for a hero. A value of 0 states that there is no /// maximum level for heroes. This is the default value. ui8 levelLimit; diff --git a/lib/mapping/MapFormatH3M.cpp b/lib/mapping/MapFormatH3M.cpp index aabe26abc..c9a2a9b40 100644 --- a/lib/mapping/MapFormatH3M.cpp +++ b/lib/mapping/MapFormatH3M.cpp @@ -184,10 +184,10 @@ void CMapLoaderH3M::readHeader() mapHeader->twoLevel = reader->readBool(); mapHeader->name.appendTextID(readLocalizedString("header.name")); mapHeader->description.appendTextID(readLocalizedString("header.description")); - mapHeader->difficulty = reader->readInt8(); + mapHeader->difficulty = static_cast(reader->readInt8Checked(0, 4)); if(features.levelAB) - mapHeader->levelLimit = reader->readUInt8(); + mapHeader->levelLimit = reader->readInt8Checked(0, std::min(100u, VLC->heroh->maxSupportedLevel())); else mapHeader->levelLimit = 0; @@ -218,7 +218,7 @@ void CMapLoaderH3M::readPlayerInfo() continue; } - playerInfo.aiTactic = static_cast(reader->readUInt8()); + playerInfo.aiTactic = static_cast(reader->readInt8Checked(-1, 3)); if(features.levelSOD) reader->skipUnused(1); //TODO: check meaning? @@ -261,8 +261,8 @@ void CMapLoaderH3M::readPlayerInfo() if(features.levelAB) { reader->skipUnused(1); //TODO: check meaning? - uint32_t heroCount = reader->readUInt32(); - for(int pp = 0; pp < heroCount; ++pp) + size_t heroCount = reader->readUInt32(); + for(size_t pp = 0; pp < heroCount; ++pp) { SHeroName vv; vv.heroId = reader->readHero(); @@ -274,39 +274,13 @@ void CMapLoaderH3M::readPlayerInfo() } } -enum class EVictoryConditionType : uint8_t -{ - ARTIFACT = 0, - GATHERTROOP = 1, - GATHERRESOURCE = 2, - BUILDCITY = 3, - BUILDGRAIL = 4, - BEATHERO = 5, - CAPTURECITY = 6, - BEATMONSTER = 7, - TAKEDWELLINGS = 8, - TAKEMINES = 9, - TRANSPORTITEM = 10, - HOTA_ELIMINATE_ALL_MONSTERS = 11, - HOTA_SURVIVE_FOR_DAYS = 12, - WINSTANDARD = 255 -}; - -enum class ELossConditionType : uint8_t -{ - LOSSCASTLE = 0, - LOSSHERO = 1, - TIMEEXPIRES = 2, - LOSSSTANDARD = 255 -}; - void CMapLoaderH3M::readVictoryLossConditions() { mapHeader->triggeredEvents.clear(); mapHeader->victoryMessage.clear(); mapHeader->defeatMessage.clear(); - auto vicCondition = static_cast(reader->readUInt8()); + auto vicCondition = static_cast(reader->readInt8Checked(-1, 12)); EventCondition victoryCondition(EventCondition::STANDARD_WIN); EventCondition defeatCondition(EventCondition::DAYS_WITHOUT_TOWN); @@ -395,9 +369,9 @@ void CMapLoaderH3M::readVictoryLossConditions() EventExpression::OperatorAll oper; EventCondition cond(EventCondition::HAVE_BUILDING); cond.position = reader->readInt3(); - cond.objectType = BuildingID::HALL_LEVEL(reader->readUInt8() + 1); + cond.objectType = BuildingID::HALL_LEVEL(reader->readInt8Checked(0,3) + 1); oper.expressions.emplace_back(cond); - cond.objectType = BuildingID::FORT_LEVEL(reader->readUInt8()); + cond.objectType = BuildingID::FORT_LEVEL(reader->readInt8Checked(0, 2)); oper.expressions.emplace_back(cond); specialVictory.effect.toOtherMessage.appendTextID("core.genrltxt.283"); @@ -578,7 +552,7 @@ void CMapLoaderH3M::readVictoryLossConditions() } // Read loss conditions - auto lossCond = static_cast(reader->readUInt8()); + auto lossCond = static_cast(reader->readInt8Checked(-1, 2)); if(lossCond == ELossConditionType::LOSSSTANDARD) { mapHeader->defeatIconIndex = 3; @@ -685,9 +659,9 @@ void CMapLoaderH3M::readAllowedHeroes() if(features.levelAB) { - uint32_t placeholdersQty = reader->readUInt32(); + size_t placeholdersQty = reader->readUInt32(); - for (uint32_t i = 0; i < placeholdersQty; ++i) + for (size_t i = 0; i < placeholdersQty; ++i) { auto heroID = reader->readHero(); mapHeader->reservedCampaignHeroes.insert(heroID); @@ -700,9 +674,9 @@ void CMapLoaderH3M::readDisposedHeroes() // Reading disposed heroes (20 bytes) if(features.levelSOD) { - ui8 disp = reader->readUInt8(); + size_t disp = reader->readUInt8(); map->disposedHeroes.resize(disp); - for(int g = 0; g < disp; ++g) + for(size_t g = 0; g < disp; ++g) { map->disposedHeroes[g].heroId = reader->readHero(); map->disposedHeroes[g].portrait = reader->readHeroPortrait(); @@ -801,10 +775,10 @@ void CMapLoaderH3M::readAllowedSpellsAbilities() void CMapLoaderH3M::readRumors() { - uint32_t rumorsCount = reader->readUInt32(); + size_t rumorsCount = reader->readUInt32(); assert(rumorsCount < 1000); // sanity check - for(int it = 0; it < rumorsCount; it++) + for(size_t it = 0; it < rumorsCount; it++) { Rumor ourRumor; ourRumor.name = readBasicString(); @@ -853,7 +827,7 @@ void CMapLoaderH3M::readPredefinedHeroes() for(int yy = 0; yy < howMany; ++yy) { hero->secSkills[yy].first = reader->readSkill(); - hero->secSkills[yy].second = reader->readUInt8(); + hero->secSkills[yy].second = reader->readInt8Checked(1,3); } } @@ -864,7 +838,7 @@ void CMapLoaderH3M::readPredefinedHeroes() hero->biographyCustomTextId = readLocalizedString(TextIdentifier("heroes", heroID, "biography")); // 0xFF is default, 00 male, 01 female - hero->gender = static_cast(reader->readUInt8()); + hero->gender = static_cast(reader->readInt8Checked(-1, 1)); assert(hero->gender == EHeroGender::MALE || hero->gender == EHeroGender::FEMALE || hero->gender == EHeroGender::DEFAULT); bool hasCustomSpells = reader->readBool(); @@ -910,8 +884,8 @@ void CMapLoaderH3M::loadArtifactsOfHero(CGHeroInstance * hero) // bag artifacts // number of artifacts in hero's bag - int amount = reader->readUInt16(); - for(int i = 0; i < amount; ++i) + size_t amount = reader->readUInt16(); + for(size_t i = 0; i < amount; ++i) { loadArtifactToSlot(hero, ArtifactPosition::BACKPACK_START + static_cast(hero->artifactsInBackpack.size())); } @@ -1038,33 +1012,33 @@ void CMapLoaderH3M::readBoxContent(CGPandoraBox * object, const int3 & mapPositi reward.heroExperience = reader->readUInt32(); reward.manaDiff = reader->readInt32(); - if(auto val = reader->readUInt8()) + if(auto val = reader->readInt8Checked(-3, 3)) reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven)); - if(auto val = reader->readUInt8()) + if(auto val = reader->readInt8Checked(-3, 3)) reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven)); reader->readResourses(reward.resources); for(int x = 0; x < GameConstants::PRIMARY_SKILLS; ++x) reward.primary.at(x) = reader->readUInt8(); - int gabn = reader->readUInt8(); //number of gained abilities - for(int oo = 0; oo < gabn; ++oo) + size_t gabn = reader->readUInt8(); //number of gained abilities + for(size_t oo = 0; oo < gabn; ++oo) { auto rId = reader->readSkill(); - auto rVal = reader->readUInt8(); + auto rVal = reader->readInt8Checked(1,3); reward.secondary[rId] = rVal; } - int gart = reader->readUInt8(); //number of gained artifacts - for(int oo = 0; oo < gart; ++oo) + size_t gart = reader->readUInt8(); //number of gained artifacts + for(size_t oo = 0; oo < gart; ++oo) reward.artifacts.push_back(reader->readArtifact()); - int gspel = reader->readUInt8(); //number of gained spells - for(int oo = 0; oo < gspel; ++oo) + size_t gspel = reader->readUInt8(); //number of gained spells + for(size_t oo = 0; oo < gspel; ++oo) reward.spells.push_back(reader->readSpell()); - int gcre = reader->readUInt8(); //number of gained creatures - for(int oo = 0; oo < gcre; ++oo) + size_t gcre = reader->readUInt8(); //number of gained creatures + for(size_t oo = 0; oo < gcre; ++oo) { auto rId = reader->readCreature(); auto rVal = reader->readUInt16(); @@ -1094,7 +1068,7 @@ CGObjectInstance * CMapLoaderH3M::readMonster(const int3 & mapPosition, const Ob //type will be set during initialization object->putStack(SlotID(0), hlp); - object->character = reader->readInt8(); + object->character = reader->readInt8Checked(0, 4); bool hasMessage = reader->readBool(); if(hasMessage) @@ -1192,17 +1166,17 @@ CGObjectInstance * CMapLoaderH3M::readWitchHut(const int3 & position, std::share CGObjectInstance * CMapLoaderH3M::readScholar(const int3 & position, std::shared_ptr objectTemplate) { - enum class ScholarBonusType : uint8_t { + enum class ScholarBonusType : int8_t { + RANDOM = -1, PRIM_SKILL = 0, SECONDARY_SKILL = 1, SPELL = 2, - RANDOM = 255 }; auto * object = readGeneric(position, objectTemplate); auto * rewardable = dynamic_cast(object); - uint8_t bonusTypeRaw = reader->readUInt8(); + uint8_t bonusTypeRaw = reader->readInt8Checked(-1, 2); auto bonusType = static_cast(bonusTypeRaw); auto bonusID = reader->readUInt8(); @@ -1477,7 +1451,7 @@ CGObjectInstance * CMapLoaderH3M::readBank(const int3 & mapPosition, std::shared int32_t guardsPresetIndex = reader->readInt32(); // presence of upgraded stack: -1 = random, 0 = never, 1 = always - int8_t upgradedStackPresence = reader->readInt8(); + int8_t upgradedStackPresence = reader->readInt8Checked(-1, 1); assert(vstd::iswithin(guardsPresetIndex, -1, 4)); assert(vstd::iswithin(upgradedStackPresence, -1, 1)); @@ -1807,7 +1781,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec for(int i = 0; i < skillsCount; ++i) { object->secSkills[i].first = reader->readSkill(); - object->secSkills[i].second = reader->readUInt8(); + object->secSkills[i].second = reader->readInt8Checked(1,3); } } @@ -1815,7 +1789,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec if(hasGarison) readCreatureSet(object, 7); - object->formation = static_cast(reader->readUInt8()); + object->formation = static_cast(reader->readInt8Checked(0, 1)); assert(object->formation == EArmyFormation::LOOSE || object->formation == EArmyFormation::TIGHT); loadArtifactsOfHero(object); @@ -1828,7 +1802,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec if(hasCustomBiography) object->biographyCustomTextId = readLocalizedString(TextIdentifier("heroes", object->subID, "biography")); - object->gender = static_cast(reader->readUInt8()); + object->gender = static_cast(reader->readInt8Checked(-1, 1)); assert(object->gender == EHeroGender::MALE || object->gender == EHeroGender::FEMALE || object->gender == EHeroGender::DEFAULT); } else @@ -1938,30 +1912,12 @@ enum class ESeerHutRewardType : uint8_t CREATURE = 10, }; -enum class EQuestMission { - NONE = 0, - LEVEL = 1, - PRIMARY_SKILL = 2, - KILL_HERO = 3, - KILL_CREATURE = 4, - ARTIFACT = 5, - ARMY = 6, - RESOURCES = 7, - HERO = 8, - PLAYER = 9, - HOTA_MULTI = 10, - // end of H3 missions - KEYMASTER = 100, - HOTA_HERO_CLASS = 101, - HOTA_REACH_DATE = 102 -}; - void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, const ObjectInstanceID & idToBeGiven) { EQuestMission missionType = EQuestMission::NONE; if(features.levelAB) { - missionType = static_cast(readQuest(hut, position)); + missionType = readQuest(hut, position); } else { @@ -1981,7 +1937,7 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con if(missionType != EQuestMission::NONE) { - auto rewardType = static_cast(reader->readUInt8()); + auto rewardType = static_cast(reader->readInt8Checked(0, 10)); Rewardable::VisitInfo vinfo; auto & reward = vinfo.reward; switch(rewardType) @@ -2003,36 +1959,34 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con } case ESeerHutRewardType::MORALE: { - reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, reader->readUInt8(), BonusSourceID(idToBeGiven)); + reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven)); break; } case ESeerHutRewardType::LUCK: { - reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, reader->readUInt8(), BonusSourceID(idToBeGiven)); + reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven)); break; } case ESeerHutRewardType::RESOURCES: { - auto rId = reader->readUInt8(); + auto rId = reader->readGameResID(); auto rVal = reader->readUInt32(); - assert(rId < features.resourcesCount); - reward.resources[rId] = rVal; break; } case ESeerHutRewardType::PRIMARY_SKILL: { - auto rId = reader->readUInt8(); + auto rId = reader->readPrimary(); auto rVal = reader->readUInt8(); - reward.primary.at(rId) = rVal; + reward.primary.at(rId.getNum()) = rVal; break; } case ESeerHutRewardType::SECONDARY_SKILL: { auto rId = reader->readSkill(); - auto rVal = reader->readUInt8(); + auto rVal = reader->readInt8Checked(1,3); reward.secondary[rId] = rVal; break; @@ -2071,11 +2025,11 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con } } -int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position) +EQuestMission CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position) { - auto missionId = reader->readUInt8(); + auto missionId = static_cast(reader->readInt8Checked(0, 10)); - switch(static_cast(missionId)) + switch(missionId) { case EQuestMission::NONE: return missionId; @@ -2100,8 +2054,8 @@ int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position) } case EQuestMission::ARTIFACT: { - int artNumber = reader->readUInt8(); - for(int yy = 0; yy < artNumber; ++yy) + size_t artNumber = reader->readUInt8(); + for(size_t yy = 0; yy < artNumber; ++yy) { auto artid = reader->readArtifact(); guard->quest->mission.artifacts.push_back(artid); @@ -2111,9 +2065,9 @@ int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position) } case EQuestMission::ARMY: { - int typeNumber = reader->readUInt8(); + size_t typeNumber = reader->readUInt8(); guard->quest->mission.creatures.resize(typeNumber); - for(int hh = 0; hh < typeNumber; ++hh) + for(size_t hh = 0; hh < typeNumber; ++hh) { guard->quest->mission.creatures[hh].type = reader->readCreature().toCreature(); guard->quest->mission.creatures[hh].count = reader->readUInt16(); @@ -2143,7 +2097,7 @@ int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position) if(missionSubID == 0) { - missionId = int(EQuestMission::HOTA_HERO_CLASS); + missionId = EQuestMission::HOTA_HERO_CLASS; std::set heroClasses; reader->readBitmaskHeroClassesSized(heroClasses, false); for(auto & hc : heroClasses) @@ -2152,7 +2106,7 @@ int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position) } if(missionSubID == 1) { - missionId = int(EQuestMission::HOTA_REACH_DATE); + missionId = EQuestMission::HOTA_REACH_DATE; guard->quest->mission.daysPassed = reader->readUInt32() + 1; break; } @@ -2194,7 +2148,7 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt if(hasGarrison) readCreatureSet(object, 7); - object->formation = static_cast(reader->readUInt8()); + object->formation = static_cast(reader->readInt8Checked(0, 1)); assert(object->formation == EArmyFormation::LOOSE || object->formation == EArmyFormation::TIGHT); bool hasCustomBuildings = reader->readBool(); @@ -2252,7 +2206,7 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt else event.humanAffected = true; - event.computerAffected = reader->readUInt8(); + event.computerAffected = reader->readBool(); event.firstOccurence = reader->readUInt16(); event.nextOccurence = reader->readUInt8(); diff --git a/lib/mapping/MapFormatH3M.h b/lib/mapping/MapFormatH3M.h index a0064bdca..84100c45a 100644 --- a/lib/mapping/MapFormatH3M.h +++ b/lib/mapping/MapFormatH3M.h @@ -35,6 +35,50 @@ class SpellID; class PlayerColor; class int3; +enum class EQuestMission { + NONE = 0, + LEVEL = 1, + PRIMARY_SKILL = 2, + KILL_HERO = 3, + KILL_CREATURE = 4, + ARTIFACT = 5, + ARMY = 6, + RESOURCES = 7, + HERO = 8, + PLAYER = 9, + HOTA_MULTI = 10, + // end of H3 missions + KEYMASTER = 100, + HOTA_HERO_CLASS = 101, + HOTA_REACH_DATE = 102 +}; + +enum class EVictoryConditionType : int8_t +{ + WINSTANDARD = -1, + ARTIFACT = 0, + GATHERTROOP = 1, + GATHERRESOURCE = 2, + BUILDCITY = 3, + BUILDGRAIL = 4, + BEATHERO = 5, + CAPTURECITY = 6, + BEATMONSTER = 7, + TAKEDWELLINGS = 8, + TAKEMINES = 9, + TRANSPORTITEM = 10, + HOTA_ELIMINATE_ALL_MONSTERS = 11, + HOTA_SURVIVE_FOR_DAYS = 12 +}; + +enum class ELossConditionType : int8_t +{ + LOSSSTANDARD = -1, + LOSSCASTLE = 0, + LOSSHERO = 1, + TIMEEXPIRES = 2 +}; + class DLL_LINKAGE CMapLoaderH3M : public IMapLoader { public: @@ -204,7 +248,7 @@ private: * * @param guard the quest guard where that quest should be applied to */ - int readQuest(IQuestObject * guard, const int3 & position); + EQuestMission readQuest(IQuestObject * guard, const int3 & position); void readSeerHutQuest(CGSeerHut * hut, const int3 & position, const ObjectInstanceID & idToBeGiven); diff --git a/lib/mapping/MapReaderH3M.cpp b/lib/mapping/MapReaderH3M.cpp index 2714e19c8..d2db92dff 100644 --- a/lib/mapping/MapReaderH3M.cpp +++ b/lib/mapping/MapReaderH3M.cpp @@ -183,6 +183,13 @@ RiverId MapReaderH3M::readRiver() return result; } +PrimarySkill MapReaderH3M::readPrimary() +{ + PrimarySkill result(readUInt8()); + assert(result <= PrimarySkill::KNOWLEDGE ); + return result; +} + SecondarySkill MapReaderH3M::readSkill() { SecondarySkill result(readUInt8()); @@ -400,32 +407,35 @@ bool MapReaderH3M::readBool() return result != 0; } -ui8 MapReaderH3M::readUInt8() +int8_t MapReaderH3M::readInt8Checked(int8_t lowerLimit, int8_t upperLimit) +{ + int8_t result = readInt8(); + assert(result >= lowerLimit); + assert(result <= upperLimit); + return std::clamp(result, lowerLimit, upperLimit); +} + +uint8_t MapReaderH3M::readUInt8() { return reader->readUInt8(); } -si8 MapReaderH3M::readInt8() +int8_t MapReaderH3M::readInt8() { return reader->readInt8(); } -ui16 MapReaderH3M::readUInt16() +uint16_t MapReaderH3M::readUInt16() { return reader->readUInt16(); } -si16 MapReaderH3M::readInt16() -{ - return reader->readInt16(); -} - -ui32 MapReaderH3M::readUInt32() +uint32_t MapReaderH3M::readUInt32() { return reader->readUInt32(); } -si32 MapReaderH3M::readInt32() +int32_t MapReaderH3M::readInt32() { return reader->readInt32(); } @@ -435,9 +445,4 @@ std::string MapReaderH3M::readBaseString() return reader->readBaseString(); } -CBinaryReader & MapReaderH3M::getInternalReader() -{ - return *reader; -} - VCMI_LIB_NAMESPACE_END diff --git a/lib/mapping/MapReaderH3M.h b/lib/mapping/MapReaderH3M.h index e2b5e01b8..42b446ba5 100644 --- a/lib/mapping/MapReaderH3M.h +++ b/lib/mapping/MapReaderH3M.h @@ -40,6 +40,7 @@ public: TerrainId readTerrain(); RoadId readRoad(); RiverId readRiver(); + PrimarySkill readPrimary(); SecondarySkill readSkill(); SpellID readSpell(); SpellID readSpell32(); @@ -70,17 +71,17 @@ public: bool readBool(); - ui8 readUInt8(); - si8 readInt8(); - ui16 readUInt16(); - si16 readInt16(); - ui32 readUInt32(); - si32 readInt32(); + uint8_t readUInt8(); + int8_t readInt8(); + int8_t readInt8Checked(int8_t lowerLimit, int8_t upperLimit); + + uint16_t readUInt16(); + + uint32_t readUInt32(); + int32_t readInt32(); std::string readBaseString(); - CBinaryReader & getInternalReader(); - private: template Identifier remapIdentifier(const Identifier & identifier); diff --git a/lib/rmg/CMapGenerator.cpp b/lib/rmg/CMapGenerator.cpp index 807090d78..02543f967 100644 --- a/lib/rmg/CMapGenerator.cpp +++ b/lib/rmg/CMapGenerator.cpp @@ -434,7 +434,7 @@ void CMapGenerator::addHeaderInfo() m.twoLevel = mapGenOptions.getHasTwoLevels(); m.name.appendLocalString(EMetaText::GENERAL_TXT, 740); m.description.appendRawString(getMapDescription()); - m.difficulty = 1; + m.difficulty = EMapDifficulty::NORMAL; addPlayerInfo(); m.waterMap = (mapGenOptions.getWaterContent() != EWaterContent::EWaterContent::NONE); m.banWaterContent(); From 9e09fe08e1a0ab2d80c722b6a09f46fbfa3724c4 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 5 Feb 2024 21:55:48 +0200 Subject: [PATCH 06/10] Fixed duplicated hero check - was used too early, before hero type is loaded --- lib/mapObjects/CGHeroInstance.cpp | 22 ---------------------- lib/mapping/MapFormatJson.cpp | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/lib/mapObjects/CGHeroInstance.cpp b/lib/mapObjects/CGHeroInstance.cpp index 53bb5c701..43889275f 100644 --- a/lib/mapObjects/CGHeroInstance.cpp +++ b/lib/mapObjects/CGHeroInstance.cpp @@ -1524,28 +1524,6 @@ std::string CGHeroInstance::getHeroTypeName() const void CGHeroInstance::afterAddToMap(CMap * map) { - if(ID != Obj::RANDOM_HERO) - { - auto existingHero = std::find_if(map->objects.begin(), map->objects.end(), [&](const CGObjectInstance * o) ->bool - { - return o && (o->ID == Obj::HERO || o->ID == Obj::PRISON) && o->subID == subID && o != this; - }); - - if(existingHero != map->objects.end()) - { - if(settings["session"]["editor"].Bool()) - { - logGlobal->warn("Hero is already on the map at %s", (*existingHero)->visitablePos().toString()); - } - else - { - logGlobal->error("Hero is already on the map at %s", (*existingHero)->visitablePos().toString()); - - throw std::runtime_error("Hero is already on the map"); - } - } - } - if(ID != Obj::PRISON) { map->heroesOnMap.emplace_back(this); diff --git a/lib/mapping/MapFormatJson.cpp b/lib/mapping/MapFormatJson.cpp index 9fa8375fe..90ad01008 100644 --- a/lib/mapping/MapFormatJson.cpp +++ b/lib/mapping/MapFormatJson.cpp @@ -1156,6 +1156,21 @@ void CMapLoaderJson::readObjects() { return a->getObjTypeIndex() < b->getObjTypeIndex(); }); + + + std::set debugHeroesOnMap; + for (auto const & object : map->objects) + { + if(object->ID != Obj::HERO && object->ID != Obj::PRISON) + continue; + + auto * hero = dynamic_cast(object.get()); + + if (debugHeroesOnMap.count(hero->getHeroType())) + logGlobal->error("Hero is already on the map at %s", hero->visitablePos().toString()); + + debugHeroesOnMap.insert(hero->getHeroType()); + } } void CMapLoaderJson::readTranslations() From a18f9d1e8d88f62129f764c9f2c8246c6e152890 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 5 Feb 2024 21:56:06 +0200 Subject: [PATCH 07/10] Added workaround for references to old 'torosar ' ID --- config/heroes/tower.json | 1 + lib/CHeroHandler.cpp | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/config/heroes/tower.json b/config/heroes/tower.json index 0fecfc54b..4ddc15e86 100644 --- a/config/heroes/tower.json +++ b/config/heroes/tower.json @@ -70,6 +70,7 @@ "torosar": { "index": 36, + "compatibilityIdentifiers" : [ "torosar " ], "class" : "alchemist", "female": false, "spellbook": [ "magicArrow" ], diff --git a/lib/CHeroHandler.cpp b/lib/CHeroHandler.cpp index a882cda0b..b8ba71690 100644 --- a/lib/CHeroHandler.cpp +++ b/lib/CHeroHandler.cpp @@ -326,6 +326,8 @@ CHeroClass * CHeroClassHandler::loadFromJson(const std::string & scope, const Js { JsonNode classConf = node["mapObject"]; classConf["heroClass"].String() = identifier; + if (!node["compatibilityIdentifiers"].isNull()) + classConf["compatibilityIdentifiers"] = node["compatibilityIdentifiers"]; classConf.setMeta(scope); VLC->objtypeh->loadSubObject(identifier, classConf, index, heroClass->getIndex()); }); @@ -756,6 +758,9 @@ void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNod objects.emplace_back(object); registerObject(scope, "hero", name, object->getIndex()); + + for(const auto & compatID : data["compatibilityIdentifiers"].Vector()) + registerObject(scope, "hero", compatID.String(), object->getIndex()); } void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNode & data, size_t index) @@ -767,6 +772,8 @@ void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNod objects[index] = object; registerObject(scope, "hero", name, object->getIndex()); + for(const auto & compatID : data["compatibilityIdentifiers"].Vector()) + registerObject(scope, "hero", compatID.String(), object->getIndex()); } ui32 CHeroHandler::level (TExpType experience) const From 1cecaf2bf5f8f0ef74dd78629bca5ca1d06c01d6 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 5 Feb 2024 22:26:53 +0200 Subject: [PATCH 08/10] Skip unresolved identifier from list of allowed heroes/artifacts in vmap --- lib/serializer/JsonSerializeFormat.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/serializer/JsonSerializeFormat.cpp b/lib/serializer/JsonSerializeFormat.cpp index 4da52c445..bbf576a96 100644 --- a/lib/serializer/JsonSerializeFormat.cpp +++ b/lib/serializer/JsonSerializeFormat.cpp @@ -135,9 +135,16 @@ void JsonSerializeFormat::readLICPart(const JsonNode & part, const JsonSerialize { const std::string & identifier = index.String(); - const si32 rawId = decoder(identifier); - if(rawId != -1) + try + { + const si32 rawId = decoder(identifier); value.insert(rawId); + } + catch (const IdentifierResolutionException & e) + { + // downgrade exception to warning (printed as part of exception generation + // this is usually caused by loading allowed heroes / artifacts list from vmap's + } } } From fd17133da333219715bb991be999c1c99ab60f00 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Tue, 6 Feb 2024 00:39:05 +0200 Subject: [PATCH 09/10] Fix editor build --- lib/mapping/CMap.cpp | 8 ++++++-- mapeditor/mapsettings/generalsettings.cpp | 20 ++++++++++---------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/mapping/CMap.cpp b/lib/mapping/CMap.cpp index c0d97a0c0..b6b520f13 100644 --- a/lib/mapping/CMap.cpp +++ b/lib/mapping/CMap.cpp @@ -42,8 +42,12 @@ DisposedHero::DisposedHero() : heroId(0), portrait(255) } -CMapEvent::CMapEvent() : players(0), humanAffected(0), computerAffected(0), - firstOccurence(0), nextOccurence(0) +CMapEvent::CMapEvent() + : players(0) + , humanAffected(false) + , computerAffected(false) + , firstOccurence(0) + , nextOccurence(0) { } diff --git a/mapeditor/mapsettings/generalsettings.cpp b/mapeditor/mapsettings/generalsettings.cpp index a15599e84..74f405232 100644 --- a/mapeditor/mapsettings/generalsettings.cpp +++ b/mapeditor/mapsettings/generalsettings.cpp @@ -35,23 +35,23 @@ void GeneralSettings::initialize(MapController & c) //set difficulty switch(controller->map()->difficulty) { - case 0: + case EMapDifficulty::EASY: ui->diffRadio1->setChecked(true); break; - case 1: + case EMapDifficulty::NORMAL: ui->diffRadio2->setChecked(true); break; - case 2: + case EMapDifficulty::HARD: ui->diffRadio3->setChecked(true); break; - case 3: + case EMapDifficulty::EXPERT: ui->diffRadio4->setChecked(true); break; - case 4: + case EMapDifficulty::IMPOSSIBLE: ui->diffRadio5->setChecked(true); break; }; @@ -67,11 +67,11 @@ void GeneralSettings::update() controller->map()->levelLimit = 0; //set difficulty - if(ui->diffRadio1->isChecked()) controller->map()->difficulty = 0; - if(ui->diffRadio2->isChecked()) controller->map()->difficulty = 1; - if(ui->diffRadio3->isChecked()) controller->map()->difficulty = 2; - if(ui->diffRadio4->isChecked()) controller->map()->difficulty = 3; - if(ui->diffRadio5->isChecked()) controller->map()->difficulty = 4; + if(ui->diffRadio1->isChecked()) controller->map()->difficulty = EMapDifficulty::EASY; + if(ui->diffRadio2->isChecked()) controller->map()->difficulty = EMapDifficulty::NORMAL; + if(ui->diffRadio3->isChecked()) controller->map()->difficulty = EMapDifficulty::HARD; + if(ui->diffRadio4->isChecked()) controller->map()->difficulty = EMapDifficulty::EXPERT; + if(ui->diffRadio5->isChecked()) controller->map()->difficulty = EMapDifficulty::IMPOSSIBLE; } void GeneralSettings::on_heroLevelLimitCheck_toggled(bool checked) From 18f9d29fd23224c8d421d689d112f354e34d855f Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Tue, 6 Feb 2024 16:36:57 +0200 Subject: [PATCH 10/10] Fix typo --- lib/mapObjects/CGHeroInstance.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mapObjects/CGHeroInstance.cpp b/lib/mapObjects/CGHeroInstance.cpp index 43889275f..d9ad418a7 100644 --- a/lib/mapObjects/CGHeroInstance.cpp +++ b/lib/mapObjects/CGHeroInstance.cpp @@ -1111,7 +1111,7 @@ std::string CGHeroInstance::getClassNameTextID() const { if (isCampaignGem()) return "core.genrltxt.735"; - return type->heroClass->getNameTranslated(); + return type->heroClass->getNameTextID(); } std::string CGHeroInstance::getNameTextID() const